From 1bad4c295cfa03f1fcb15930ca972f5fd8322429 Mon Sep 17 00:00:00 2001 From: cisco Date: Mon, 19 Oct 2026 16:47:12 +0100 Subject: [PATCH] feat: budget the remaining sensitive routes, filter the history endpoints, consolidate XDR validation (#682, #692, #693, #694) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four issues, three distinct gaps left by the earlier work in each area. Rate limiting (#682, #693). The tiers for SEP-10 auth, settlement submission, and anchor initiation were already in place; what the policy table did not model was a second group of routes whose cost is not in their response. Four new policies, each with its own key prefix and env-tunable budget: `authLogout` (the route answers `ok` without a valid token by design, so the revoke-and-audit path is reachable anonymously), `expenseMutate` (editing or deleting an expense rewrites every participant's obligations, while only creation was budgeted), `treasuryConfig` (enable and validate-signers each read live account state from Horizon and decide who may move group funds), and `webhookTest` (one inbound request becomes an outbound delivery with retries). `GET /withdraw/:id` takes the existing `anchorPoll` budget — it was the one anchor-transaction read still on the global bucket. Pagination and filtering (#692). Two list endpoints answered with unbounded `findMany` queries; the webhook delivery list in particular grows by one row per event per endpoint per retry, so it grew without bound in the database scan, the serialization, and the response body at once. Both now use the shared `(createdAt, id)` cursor contract and return standard `meta`, and the delivery list stops returning the stored request payload and the receiver's response text. Filtering was the other half: settlements by public status, asset, and date range; the ledger by entry type and date range; deliveries by status and event. Every filter is pushed into the query rather than applied to a fetched page — filtering after the merge would return pages shorter than the requested limit and make `hasMore` describe the unfiltered scan. XDR validation (#694). `src/utils/stellar-xdr.ts` carried a second intent validator that was weaker than the submission path's: it never verified signatures or sequence numbers, compared amounts as raw strings (so "10" did not match "10.0000000"), parsed the envelope three times, and reported a fee-bump wrapper as "must contain exactly one operation". It had no callers outside its own test. Removed, and the intent matcher in src/services/stellar.ts refactored into a pure, field-reporting `matchPaymentIntent` that both the throwing submission path and the new non-throwing `src/services/xdr-inspection.ts` compose — so there is one description of what "matches the intent" means, and signature verification is on by default rather than absent. Also documents the `sourceSequence` expectation's off-by-one against `buildPayment`'s account sequence, which nothing populates today and would silently reject every envelope if it ever were. Verification: `npm test` 4252 passed / 22 skipped across 207 files (up from 4170/205), `npm run build` clean, no new lint warnings. Co-Authored-By: Claude Opus 5 --- .env.example | 19 + CHANGELOG.md | 14 + README.md | 45 +- docs/api-contract.md | 75 +- src/config.ts | 28 + src/lib/rate-limit.ts | 53 ++ src/routes/auth.ts | 5 + src/routes/expenses.ts | 9 + src/routes/settlements.ts | 94 ++- src/routes/treasury.ts | 7 + src/routes/webhooks.ts | 46 +- src/routes/withdraw.ts | 5 + src/services/ledger.ts | 120 ++++ src/services/settlement-history.ts | 243 +++++++ src/services/stellar.ts | 144 +++- src/services/webhook-listing.ts | 192 +++++ src/services/xdr-inspection.ts | 247 +++++++ src/utils/__tests__/stellar-xdr.test.ts | 129 +--- src/utils/stellar-xdr.ts | 92 +-- tests/rate-limit-sensitive-routes.test.ts | 199 ++++++ tests/rate-limit-wiring.test.ts | 22 + .../history-pagination-filtering.test.ts | 672 ++++++++++++++++++ tests/services/xdr-inspection.test.ts | 417 +++++++++++ 23 files changed, 2586 insertions(+), 291 deletions(-) create mode 100644 src/services/ledger.ts create mode 100644 src/services/settlement-history.ts create mode 100644 src/services/webhook-listing.ts create mode 100644 src/services/xdr-inspection.ts create mode 100644 tests/routes/history-pagination-filtering.test.ts create mode 100644 tests/services/xdr-inspection.test.ts diff --git a/.env.example b/.env.example index 145aa6d4..d3de7b27 100644 --- a/.env.example +++ b/.env.example @@ -90,6 +90,25 @@ RATE_LIMIT_GROUP=10 RATE_LIMIT_GROUP_WINDOW_MS=60000 RATE_LIMIT_HISTORY=30 RATE_LIMIT_HISTORY_WINDOW_MS=60000 +# Session teardown. Keyed by IP, because POST /auth/logout deliberately answers +# `ok` for an absent or invalid token — it must not be an oracle for token +# validity — while still revoking refresh tokens and writing an audit row. +RATE_LIMIT_AUTH_LOGOUT_MAX=20 +RATE_LIMIT_AUTH_LOGOUT_WINDOW_MS=60000 +# Editing or deleting an expense rewrites every other participant's +# obligations. Its own bucket, so correcting a typo never exhausts the budget +# that guards opening new obligations. +RATE_LIMIT_EXPENSE_MUTATE_MAX=30 +RATE_LIMIT_EXPENSE_MUTATE_WINDOW_MS=60000 +# Treasury signer configuration (enable, validate-signers): each call reads +# live account state from Horizon, and both decide who may move group funds. +RATE_LIMIT_TREASURY_CONFIG_MAX=10 +RATE_LIMIT_TREASURY_CONFIG_WINDOW_MS=60000 +# Firing a test webhook turns one inbound request into an outbound delivery on +# the same retry schedule as a real event, so this is the tightest write +# budget — a test fire confirms wiring once, never in a loop. +RATE_LIMIT_WEBHOOK_TEST_MAX=5 +RATE_LIMIT_WEBHOOK_TEST_WINDOW_MS=60000 # Inbound SEP-24 anchor callbacks (POST /api/webhooks/sep24). # diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cb50dde..a44c8f99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **Rate-limit tiers for the sensitive routes still on the global bucket (#682, #693)** — Four new policies in `src/lib/rate-limit.ts`, each with its own key prefix and env-tunable budget: `authLogout` (`POST /auth/logout`, IP-keyed — the route answers `ok` without a valid token, so the revoke-and-audit path is reachable anonymously), `expenseMutate` (`PATCH`/`DELETE /expenses/:id`, which rewrite every participant's obligations while only creation was budgeted), `treasuryConfig` (`POST /groups/:id/treasury/enable` and `.../validate-signers`, one Horizon account read per call), and `webhookTest` (`POST /groups/:groupId/webhooks/:webhookId/test`, which turns one inbound request into an outbound delivery with retries). `GET /withdraw/:id` now takes the existing `anchorPoll` budget — it was the one anchor-transaction read left unbudgeted. +- **Filtering on the group transaction-history endpoints (#692)** — `GET /groups/:id/settlements` takes `status` (the public settlement status, including the two values derived from the signing deadline rather than stored), `asset`, `startDate`/`endDate`, and `includeTotal`; `GET /groups/:id/ledger` takes `type` (which selects which of the three tables are read at all) and a date range; `GET /groups/:groupId/webhooks/:webhookId/deliveries` takes `status` and `event`. Every filter is pushed into the query rather than applied to a fetched page, so pages stay full and `hasMore` stays exact. +- **Non-throwing signed-XDR inspection (#694)** — `src/services/xdr-inspection.ts` reports which field of an envelope diverged from a stored intent (`{ ok: false, code, field, message }`) instead of raising, for callers that need to classify an envelope rather than reject a request. It composes the same primitives the submission path uses, so there is one description of what "matches the intent" means. + +### Changed + +- **Cursor pagination on the two unbounded list endpoints (#692)** — `GET /groups/:groupId/webhooks/:webhookId/deliveries` and `GET /groups/:groupId/webhooks` answered with unbounded `findMany` queries. Both now use the shared `(createdAt, id)` cursor contract and return standard `meta`. The delivery list also stops returning the stored request payload and the receiver's response body, which were never appropriate for a list response. + +### Removed + +- **The duplicate XDR intent validator in `src/utils/stellar-xdr.ts` (#694)** — `validateTransactionIntent` was a second, weaker copy of the submission path's rules: it never verified signatures or sequence numbers, compared amounts as raw strings (so `"10"` did not match `"10.0000000"`), and reported a fee-bump envelope as "must contain exactly one operation". It had no callers outside its own test. `isValidXdr`, which production code does use, is unchanged. + ### Verified - **Debt simplification test coverage (#685)** — Comprehensive test suites already verify greedy debt-simplification algorithm across circular debts (balanced/uneven rings, bidirectional netting), zero-balance members, unequal shares, and multi-asset segregation. 61 tests across `tests/settlement.test.ts` (lines 287-454) and `tests/multi-asset-settlement.test.ts` enforce mathematical completeness and (n-1) transfer bound. diff --git a/README.md b/README.md index 5482bab4..17f443e8 100644 --- a/README.md +++ b/README.md @@ -320,18 +320,22 @@ traffic pattern or trust boundary replace that default with their own bucket: | --- | --- | --- | | `POST /auth/challenge` | `RATE_LIMIT_AUTH_CHALLENGE_MAX` / `_WINDOW_MS` | 20 / 1 min | | `POST /auth/verify`, `POST /auth/refresh` | `RATE_LIMIT_AUTH_VERIFY_MAX` / `_WINDOW_MS` | 10 / 1 min | +| `POST /auth/logout` | `RATE_LIMIT_AUTH_LOGOUT_MAX` / `_WINDOW_MS` | 20 / 1 min | | `POST /groups/:id/expenses` | `RATE_LIMIT_EXPENSE_CREATE_MAX` / `_WINDOW_MS` | 30 / 1 min | +| `PATCH /expenses/:id`, `DELETE /expenses/:id` | `RATE_LIMIT_EXPENSE_MUTATE_MAX` / `_WINDOW_MS` | 30 / 1 min | | `POST /expenses/:id/settle`, `POST /groups/:id/settlements`, `POST /groups/:id/treasury/deposit`, `POST /groups/:id/treasury/withdraw` | `RATE_LIMIT_SETTLEMENT_CREATE_MAX` / `_WINDOW_MS` | 20 / 1 min | | `POST /settlements/:id/confirm`, `POST /withdraw/:id/confirm` | `RATE_LIMIT_SETTLEMENT_CONFIRM_MAX` / `_WINDOW_MS` | 20 / 1 min | | `POST /api/v1/settlements/execute` | `RATE_LIMIT_SETTLEMENT_EXECUTE_MAX` / `_WINDOW_MS` | 20 / 1 min | | `POST /treasury-transactions/:id/confirm`, `POST /groups/:groupId/treasury/proposals/:proposalId/sign`, `POST /api/v1/treasury/proposals/:id/signatures` | `RATE_LIMIT_TREASURY_SUBMIT_MAX` / `_WINDOW_MS` | 30 / 1 min | | `POST /groups/:groupId/treasury/proposals`, `POST /api/v1/treasury/proposals` | `RATE_LIMIT_TREASURY_PROPOSE_MAX` / `_WINDOW_MS` | 20 / 1 min | +| `POST /groups/:id/treasury/enable`, `POST /groups/:id/treasury/validate-signers` | `RATE_LIMIT_TREASURY_CONFIG_MAX` / `_WINDOW_MS` | 10 / 1 min | | `POST /anchors/deposit`, `POST /anchors/withdraw`, `POST /anchors/sessions/:id/complete`, `POST /api/v1/sep24/deposit`, `POST /api/v1/sep24/withdraw`, `POST /withdraw` | `RATE_LIMIT_ANCHOR_INIT_MAX` / `_WINDOW_MS` | 10 / 1 min | -| `GET /anchors`, `GET /anchors/sessions`, `GET /anchors/sessions/:id` | `RATE_LIMIT_ANCHOR_POLL_MAX` / `_WINDOW_MS` | 60 / 1 min | +| `GET /anchors`, `GET /anchors/sessions`, `GET /anchors/sessions/:id`, `GET /api/v1/sep24/transaction`, `GET /api/v1/sep24/transactions`, `GET /withdraw/:id` | `RATE_LIMIT_ANCHOR_POLL_MAX` / `_WINDOW_MS` | 60 / 1 min | | `POST /anchors/webhook` | `RATE_LIMIT_ANCHOR_WEBHOOK_MAX` / `_WINDOW_MS` | 50 / 1 min | | `POST /api/v1/sep24/callback`, `POST /api/v1/webhooks/sep24` | `SEP24_RATE_LIMIT_MAX` / `_WINDOW_MS` | 10 / 1 min | | `POST /groups` | `RATE_LIMIT_GROUP` / `_WINDOW_MS` | 10 / 1 min | | `GET /history` | `RATE_LIMIT_HISTORY` / `_WINDOW_MS` | 30 / 1 min | +| `POST /groups/:groupId/webhooks/:webhookId/test` | `RATE_LIMIT_WEBHOOK_TEST_MAX` / `_WINDOW_MS` | 5 / 1 min | **Tuning a deployment.** Every value above is an environment variable with a safe default, so a deployment overrides only what it needs — for example a @@ -631,11 +635,46 @@ ordering coordinates, never a group or user id: access is decided by each query's own scope plus its membership check, so a cursor from one resource replayed against another cannot widen what the caller can read. -Covers `GET /groups`, `/groups/:id/expenses`, `/groups/:id/ledger`, -`/groups/:id/treasury/history`, `/anchors/sessions`, and `/history` (which +Covers `GET /groups`, `/groups/:id`, `/groups/:id/expenses`, +`/groups/:id/settlements`, `/groups/:id/ledger`, +`/groups/:id/treasury/history`, `/groups/:groupId/treasury/proposals`, +`/groups/:groupId/webhooks`, `/groups/:groupId/webhooks/:webhookId/deliveries`, +`/anchors/sessions`, `/api/v1/sep24/transactions`, and `/history` (which paginates its expense and settlement streams independently, via `cursor` and `settlementCursor`). +### Filtering the transaction history + +The history endpoints take filters alongside the pagination parameters above. +Filters are pushed into the database query, never applied to an already-fetched +page: a filter applied after the fact would return pages shorter than the +requested `limit` and make `hasMore` describe the unfiltered scan. + +| Endpoint | Filters | +| --- | --- | +| `GET /groups/:id/expenses` | `asset` (`XLM`\|`USDC`), `status` (`PENDING`\|`SETTLED`), `startDate`, `endDate`, `includeTotal` | +| `GET /groups/:id/settlements` | `status` (`awaiting_signature`\|`submitted`\|`confirmed`\|`failed`\|`expired`), `asset`, `startDate`, `endDate`, `includeTotal` | +| `GET /groups/:id/ledger` | `type` (one or more of `expense`,`settlement`,`treasury`), `startDate`, `endDate` | +| `GET /groups/:groupId/webhooks/:webhookId/deliveries` | `status` (`pending`\|`delivered`\|`failed`), `event` | +| `GET /groups/:groupId/audit-logs` | `action`, `actorUserId`, `startDate`, `endDate` | + +Notes that matter when integrating: + +- **Dates** are ISO-8601 with an offset, and both bounds are inclusive. A + reversed range is `INVALID_RANGE`, not an empty page. +- **Unknown filter values** are `VALIDATION_ERROR`. A typo never silently + returns nothing. +- **`includeTotal=true`** adds `meta.total`, the count of the *filtered* set + ignoring the cursor. It is opt-in because counting is a second query over + every matching row, which the cursor contract otherwise never pays for. +- **`status` on settlements** filters by the same public status the settlement + status endpoint reports, including the two values derived from the signing + deadline rather than stored (`expired`, and `submitted` vs + `awaiting_signature`). The five values partition the history: every + settlement matches exactly one. +- **`type` on the ledger** decides which tables are read at all, so + `?type=expense` issues one query instead of three. + ## TypeScript strict mode The whole of `src/` compiles under TypeScript's [`strict`](https://www.typescriptlang.org/tsconfig/#strict) flag — see [tsconfig.json](tsconfig.json) for the exact configuration. `strict` turns on every strict type-checking family at once (`strictNullChecks`, `noImplicitAny`, `strictFunctionTypes`, `strictBindCallApply`, `strictPropertyInitialization`, `noImplicitThis`, `alwaysStrict`, and `useUnknownInCatchVariables`), so `null`/`undefined` flows, implicit `any`s, and unbound `this` are compile errors rather than production incidents. diff --git a/docs/api-contract.md b/docs/api-contract.md index 46f8b406..4590853d 100644 --- a/docs/api-contract.md +++ b/docs/api-contract.md @@ -82,8 +82,10 @@ name go to the log instead. ## Pagination -Applies to `GET /groups`, `/groups/:id/expenses`, `/groups/:id/settlements`, `/groups/:id/ledger`, -`/groups/:id/treasury/history`, `/anchors/sessions`, `/api/v1/sep24/transactions`, and `/history`. +Applies to `GET /groups`, `/groups/:id`, `/groups/:id/expenses`, `/groups/:id/settlements`, +`/groups/:id/ledger`, `/groups/:id/treasury/history`, `/groups/:groupId/treasury/proposals`, +`/groups/:groupId/webhooks`, `/groups/:groupId/webhooks/:webhookId/deliveries`, +`/anchors/sessions`, `/api/v1/sep24/transactions`, and `/history`. Defined in [../src/lib/pagination.ts](../src/lib/pagination.ts). ### Query parameters @@ -139,6 +141,61 @@ exactly like an unfiltered one. The count behind `includeTotal` covers the filters but not the cursor, so the total stays stable while a client pages through the result. +### Settlement list filters (`GET /groups/:id/settlements`) + +Defined in [../src/services/settlement-history.ts](../src/services/settlement-history.ts). +Same shape as the expense filters above, and the same composition with the +cursor. + +| Parameter | Type | Notes | +| --- | --- | --- | +| `status` | one of the five public settlement statuses | Filters by the value [`GET /settlements/:id/status`](#get-settlementsidstatus) reports for the same row, not by the internal column | +| `asset` | `"XLM"` \| `"USDC"` | Filter by asset code. Other codes are a `VALIDATION_ERROR` | +| `startDate` / `endDate` | ISO 8601 datetime | Inclusive `createdAt` bounds. A reversed range is `INVALID_RANGE` | +| `includeTotal` | `"true"` \| `"false"` | Opt-in `meta.total` over the filtered set, ignoring the cursor | + +Two of the five public statuses are *derived* from the signing deadline rather +than stored — a row at `pending` or `submitted` past its `expiresAt` reports as +`expired` — so the filter is written as the exact inverse of that mapping, +including its clock-skew tolerance. Consequences a client can rely on: + +- the five values **partition** the history: every settlement matches exactly + one, so no row is unreachable and none appears under two filters; +- a filtered row reports the **same status** when read individually through + `GET /settlements/:id/status`. + +### Ledger filters (`GET /groups/:id/ledger`) + +Defined in [../src/services/ledger.ts](../src/services/ledger.ts). The ledger +interleaves expenses, settlements, and treasury transactions into one +time-ordered feed. + +| Parameter | Type | Notes | +| --- | --- | --- | +| `type` | one or more of `expense`, `settlement`, `treasury` | Comma-separated (`?type=expense,settlement`). Selects which tables are read at all, so one kind means one query instead of three. An unknown kind is a `VALIDATION_ERROR` | +| `startDate` / `endDate` | ISO 8601 datetime | Inclusive `createdAt` bounds, pushed into every table read. A reversed range is `INVALID_RANGE` | + +Both filters are applied inside each table's query rather than to the merged +page. That is what keeps pages full: each table is read with its own +`limit + 1` window, so dropping rows after the merge would return short pages +and make `hasMore` describe the unfiltered scan. + +### Webhook delivery filters (`GET /groups/:groupId/webhooks/:webhookId/deliveries`) + +Defined in [../src/services/webhook-listing.ts](../src/services/webhook-listing.ts). + +| Parameter | Type | Notes | +| --- | --- | --- | +| `status` | `"pending"` \| `"delivered"` \| `"failed"` | Delivery state recorded by the worker | +| `event` | a webhook event type | Filter to one event type | + +A delivery row also stores the exact signed request body and the receiver's +response text. Neither is returned by this endpoint: the payload is the group's +own data re-served through a different authorization path, and the response body +is third-party text. The list returns `id`, `webhookId`, `eventType`, `status`, +`attempts`, `responseStatusCode`, `nextAttemptAt`, `lastAttemptAt`, `createdAt`, +and `updatedAt`. + --- ## Idempotency @@ -267,6 +324,20 @@ response or in logs. A rejected transaction is never submitted and never advances the settlement's status — the settlement stays awaiting a signature and the wallet can sign the correct envelope instead. +The same checks, in the same order, are available as a **non-throwing +inspection** in +[../src/services/xdr-inspection.ts](../src/services/xdr-inspection.ts), for +callers that want the diverging field reported rather than raised — +`inspectSignedTransaction` returns `{ ok: true, tx, hash, source, sequence }` or +`{ ok: false, code, field, message }`, where `code` is one of `xdr_malformed`, +`xdr_mismatch`, `intent_expired`, or `xdr_unsigned`, and `field` names the part +of the envelope that diverged. It adds no rules of its own: it composes the same +primitives the submission path uses, so there is exactly one description of what +"matches the intent" means. Signature verification is on by default and can only +be waived explicitly, for the paths where authorship is established elsewhere +(an unsigned intent read back, or a multisig proposal whose shared source +account never signs with its own key). + --- ## `GET /settlements/:id/status` diff --git a/src/config.ts b/src/config.ts index 8622c96c..007b89c6 100644 --- a/src/config.ts +++ b/src/config.ts @@ -414,6 +414,34 @@ const schema = z.object({ // they share with every read the same client makes. RATE_LIMIT_UPLOAD_RECEIPT_MAX: z.coerce.number().int().positive().max(100000).default(20), RATE_LIMIT_UPLOAD_RECEIPT_WINDOW_MS: z.coerce.number().int().positive().max(MAX_RATE_LIMIT_WINDOW_MS).default(60000), + // Issues #682 / #693 — the remaining sensitive routes that were still + // answering from the global bucket. Each is budgeted for the work *it* + // causes, not for the size of its response body. + // + // Logout revokes every refresh token a user holds, and the route answers + // `{ ok: true }` whether or not the bearer token was valid — deliberately, + // so it is not an oracle for token validity, but that also means an + // anonymous caller can drive the write path. Keyed by IP, like the other + // pre-session auth routes. + RATE_LIMIT_AUTH_LOGOUT_MAX: z.coerce.number().int().positive().max(100000).default(20), + RATE_LIMIT_AUTH_LOGOUT_WINDOW_MS: z.coerce.number().int().positive().max(MAX_RATE_LIMIT_WINDOW_MS).default(60000), + // Editing or deleting an expense rewrites the obligations of every other + // participant. `RATE_LIMIT_EXPENSE_CREATE_*` bounded only creation, so the + // mutations that follow it drew on the global allowance instead. + RATE_LIMIT_EXPENSE_MUTATE_MAX: z.coerce.number().int().positive().max(100000).default(30), + RATE_LIMIT_EXPENSE_MUTATE_WINDOW_MS: z.coerce.number().int().positive().max(MAX_RATE_LIMIT_WINDOW_MS).default(60000), + // Treasury signer configuration: enabling a treasury and validating a + // proposed signer set both read live account state from Horizon, so each + // call spends upstream budget, and both decide who may move group funds. + // Tight, because neither is an operation a client repeats in bursts. + RATE_LIMIT_TREASURY_CONFIG_MAX: z.coerce.number().int().positive().max(100000).default(10), + RATE_LIMIT_TREASURY_CONFIG_WINDOW_MS: z.coerce.number().int().positive().max(MAX_RATE_LIMIT_WINDOW_MS).default(60000), + // Firing a test webhook turns one inbound request into an outbound HTTP + // delivery to a caller-registered URL, on the same retry schedule as a real + // event. `RATE_LIMIT_WEBHOOK_CREATE_*` bounded registering an endpoint but + // not ringing it, which is the cheaper half of the same amplification. + RATE_LIMIT_WEBHOOK_TEST_MAX: z.coerce.number().int().positive().max(100000).default(5), + RATE_LIMIT_WEBHOOK_TEST_WINDOW_MS: z.coerce.number().int().positive().max(MAX_RATE_LIMIT_WINDOW_MS).default(60000), // trusted proxies: only trust X-Forwarded-For if the direct peer is in this // comma-separated list; otherwise Fastify falls back to req.ip = socket remote. TRUSTED_PROXY_IPS: z.string().default(""), diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index dad11c4f..51e6914e 100644 --- a/src/lib/rate-limit.ts +++ b/src/lib/rate-limit.ts @@ -53,7 +53,11 @@ export type RateLimitPolicyName = | "global" | "authChallenge" | "authVerify" + | "authLogout" | "expenseCreate" + | "expenseMutate" + | "treasuryConfig" + | "webhookTest" | "settlementCreate" | "settlementConfirm" | "settlementExecute" @@ -116,6 +120,18 @@ export function rateLimitPolicies(): Record { const { id: groupId } = idParamSchema.parse(req.params); - const { cursor, limit, order } = paginationQuerySchema.parse(req.query ?? {}); - - const position = requireCursor(cursor); - - const settlements = await prisma.settlement.findMany({ - where: { groupId, ...cursorFilter(position, order) }, - include: settlementInclude, - orderBy: cursorOrderBy(order), - take: takeForPage(limit), - }); + // Membership was checked by the route guard before any row is read, and + // the `groupId` filter the service applies is what scopes the page — + // never the cursor. See src/services/settlement-history.ts. + const query = settlementListQuerySchema.parse(req.query ?? {}); + + const { items, meta } = await listGroupSettlements( + groupId, + query, + settlementInclude + ); - const { items, meta } = buildPage(settlements, limit, order); return { settlements: items.map(serializeSettlement), meta }; }); @@ -1143,32 +1152,51 @@ export default async function settlementRoutes(app: FastifyInstance) { { preHandler: requireGroupRole("member", { param: "id" }) }, async (req) => { const { id: groupId } = idParamSchema.parse(req.params); - const { cursor, limit, order } = paginationQuerySchema.parse(req.query ?? {}); + const query = ledgerQuerySchema.parse(req.query ?? {}); + const { cursor, limit, order } = query; + assertValidLedgerRange(query.startDate, query.endDate); const position = requireCursor(cursor); - const where = { groupId, ...cursorFilter(position, order) }; + // The date bound is pushed into each table's query rather than applied to + // the merged page: each table is read with its own `limit + 1` window, so + // filtering after the merge would shrink pages and make `hasMore` + // describe the unfiltered scan. See src/services/ledger.ts. + const where = { + groupId, + ...ledgerDateFilter(query), + ...cursorFilter(position, order), + }; const orderBy = cursorOrderBy(order); const take = takeForPage(limit); + // A `type` filter selects which tables are read at all, so a request for + // one kind issues one query instead of three. An unasked-for kind + // resolves to an empty array without touching the database. const [expenses, settlements, treasuryTxs] = await Promise.all([ - prisma.expense.findMany({ - where, - include: { payer: true, shares: { include: { user: true } } }, - orderBy, - take, - }), - prisma.settlement.findMany({ - where, - include: settlementInclude, - orderBy, - take, - }), - prisma.treasuryTransaction.findMany({ - where, - include: { user: true }, - orderBy, - take, - }), + includesLedgerType(query, "expense") + ? prisma.expense.findMany({ + where, + include: { payer: true, shares: { include: { user: true } } }, + orderBy, + take, + }) + : [], + includesLedgerType(query, "settlement") + ? prisma.settlement.findMany({ + where, + include: settlementInclude, + orderBy, + take, + }) + : [], + includesLedgerType(query, "treasury") + ? prisma.treasuryTransaction.findMany({ + where, + include: { user: true }, + orderBy, + take, + }) + : [], ]); const direction = order === "desc" ? -1 : 1; diff --git a/src/routes/treasury.ts b/src/routes/treasury.ts index 1685a667..a9122340 100644 --- a/src/routes/treasury.ts +++ b/src/routes/treasury.ts @@ -57,6 +57,10 @@ export default async function treasuryRoutes(app: FastifyInstance) { app.post( "/groups/:id/treasury/enable", { + // Decides which account holds group funds, and reads live account state + // from Horizon on the way — budgeted apart from the routes that move + // the money. + ...rateLimited("treasuryConfig"), preHandler: requireGroupRole("admin", { param: "id" }), schema: { tags: ["Treasury"], @@ -168,6 +172,9 @@ export default async function treasuryRoutes(app: FastifyInstance) { app.post( "/groups/:id/treasury/validate-signers", { + // One Horizon account read per call, so this is upstream budget an + // admin session can spend without changing anything. + ...rateLimited("treasuryConfig"), preHandler: requireGroupRole("admin", { param: "id" }), schema: { tags: ["Treasury"], diff --git a/src/routes/webhooks.ts b/src/routes/webhooks.ts index d8c5293d..5738ebdd 100644 --- a/src/routes/webhooks.ts +++ b/src/routes/webhooks.ts @@ -13,6 +13,12 @@ import { verifySep24Signature, } from "../services/sep24"; import { createWebhookSecret, dispatchWebhook } from "../services/webhook"; +import { + deliveryListQuerySchema, + listGroupWebhooks, + listWebhookDeliveries, + webhookListQuerySchema, +} from "../services/webhook-listing"; import { audit } from "../services/audit"; const paramsSchema = z.object({ groupId: z.string().min(1) }); @@ -291,15 +297,17 @@ async function webhookManagementRoutes(app: FastifyInstance) { return { webhook: publicWebhook(webhook, true) }; }); + // Paginated like every other list endpoint (#692). The endpoint count per + // group is small today, but an unbounded `findMany` is a response size no + // caller can predict, and a client that has to handle `meta` here handles + // it on the delivery list below too. app.get("/groups/:groupId/webhooks", asMember, async (req) => { const { groupId } = paramsSchema.parse(req.params); + const query = webhookListQuerySchema.parse(req.query ?? {}); - const webhooks = await (prisma as any).webhook.findMany({ - where: { groupId }, - orderBy: { createdAt: "desc" }, - }); + const { items, meta } = await listGroupWebhooks(groupId, query); - return { webhooks: webhooks.map((webhook: any) => publicWebhook(webhook)) }; + return { webhooks: items.map((webhook: any) => publicWebhook(webhook)), meta }; }); app.delete("/groups/:groupId/webhooks/:webhookId", asAdmin, async (req) => { @@ -328,7 +336,13 @@ async function webhookManagementRoutes(app: FastifyInstance) { return { deleted: true }; }); - app.post("/groups/:groupId/webhooks/:webhookId/test", asMember, async (req) => { + // Asking the server to ring a registered endpoint: one inbound request + // becomes an outbound delivery on the same retry schedule as a real event, + // so it carries the tightest write budget in the table (#693). + app.post( + "/groups/:groupId/webhooks/:webhookId/test", + { ...rateLimited("webhookTest"), ...asMember }, + async (req) => { const auth = requireUser(req); const { groupId, webhookId } = webhookParamsSchema.parse(req.params); @@ -350,8 +364,18 @@ async function webhookManagementRoutes(app: FastifyInstance) { return { queued: true }; }); + // Delivery history: one row per event per endpoint, kept for every retry, + // so this is the list that grows without bound (#692). Paged and filterable + // by delivery status and event type — the question a caller actually has is + // "what is failing to reach my endpoint", which should not require reading + // every success first. + // + // The response carries `deliverySummary` fields only: a delivery row also + // holds the signed request body and the receiver's response text, neither of + // which belongs in a list response. See src/services/webhook-listing.ts. app.get("/groups/:groupId/webhooks/:webhookId/deliveries", asMember, async (req) => { const { groupId, webhookId } = webhookParamsSchema.parse(req.params); + const query = deliveryListQuerySchema.parse(req.query ?? {}); const webhook = await (prisma as any).webhook.findFirst({ where: { id: webhookId, groupId }, @@ -359,11 +383,11 @@ async function webhookManagementRoutes(app: FastifyInstance) { }); if (!webhook) throw Errors.notFound("Webhook not found"); - const deliveries = await (prisma as any).webhookDelivery.findMany({ - where: { webhookId }, - orderBy: { createdAt: "desc" }, - }); + // Scoped by the endpoint id resolved above, which was itself scoped to the + // caller's group — the cursor only decides where inside that set the page + // starts. + const { items, meta } = await listWebhookDeliveries(webhookId, query); - return { deliveries }; + return { deliveries: items, meta }; }); } diff --git a/src/routes/withdraw.ts b/src/routes/withdraw.ts index f9847e79..31ad6051 100644 --- a/src/routes/withdraw.ts +++ b/src/routes/withdraw.ts @@ -259,6 +259,11 @@ export default async function withdrawalRoutes(app: FastifyInstance) { app.get( "/withdraw/:id", { + // The anchor-transaction status read a client polls while an + // interactive withdrawal runs, so it takes the same budget as the other + // anchor polls (#693) rather than the global one it shares with every + // unrelated read. + ...rateLimited("anchorPoll"), preHandler: [app.authenticate], schema: { tags: ["SEP-24"], diff --git a/src/services/ledger.ts b/src/services/ledger.ts new file mode 100644 index 00000000..01088218 --- /dev/null +++ b/src/services/ledger.ts @@ -0,0 +1,120 @@ +/** + * Filtering for the group ledger — the combined expense / settlement / + * treasury history (issue #692). + * + * `GET /groups/:id/ledger` interleaves three tables into one time-ordered + * feed. It paged correctly, but every request read all three tables whatever + * the caller wanted: a client rendering "recent expenses" still paid for a + * settlement scan and a treasury scan, and then threw both away. + * + * Two filters fix that, and both are cheap because they push into the queries + * rather than filtering the merged page: + * + * - `type` selects which tables are read at all. A `type=expense` request + * issues one query instead of three. + * - `startDate` / `endDate` bound `createdAt` on each table, so a request for + * last month's activity is an indexed range scan rather than a scan of + * everything newer than the cursor. + * + * Filtering the *merged* page instead would have been the trap: each table is + * read with its own `limit + 1` window, so dropping rows after the merge would + * silently shrink pages — a page of 50 could come back with 3 — and `hasMore` + * would describe the unfiltered scan. Pushing the filters into each query + * keeps every page full and `hasMore` exact. + */ +import { z } from "zod"; +import { Errors } from "../errors"; +import { paginationQuerySchema } from "../lib/pagination"; + +/** The three kinds of entry the ledger interleaves. */ +export const LEDGER_ENTRY_TYPES = ["expense", "settlement", "treasury"] as const; +export type LedgerEntryType = (typeof LEDGER_ENTRY_TYPES)[number]; + +/** + * Query parameters for the ledger. Extends the shared pagination schema, so + * cursor/limit/order are identical to every other paginated route. + * + * `type` accepts one kind or a comma-separated list (`type=expense,settlement`) + * — a single parameter rather than three booleans, so "everything" stays the + * default and an unknown kind is a validation error rather than a silently + * empty feed. + */ +export const ledgerQuerySchema = paginationQuerySchema.extend({ + type: z + .string() + .min(1) + .max(64) + .optional() + .transform((value, ctx) => { + if (value === undefined) return undefined; + const requested = value.split(",").map((part) => part.trim()); + const unknown = requested.filter( + (part) => !LEDGER_ENTRY_TYPES.includes(part as LedgerEntryType) + ); + if (unknown.length > 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `type must be one or more of: ${LEDGER_ENTRY_TYPES.join(", ")}`, + }); + return z.NEVER; + } + // De-duplicated: `type=expense,expense` asks for one kind, and a + // repeated kind must not read the same table twice. + return [...new Set(requested as LedgerEntryType[])]; + }), + startDate: z.string().datetime({ offset: true }).optional(), + endDate: z.string().datetime({ offset: true }).optional(), +}); + +export type LedgerQuery = z.infer; + +/** + * Reject a reversed range up front, so an impossible window reads as a client + * mistake rather than as an empty ledger. + */ +export function assertValidRange( + startDate: string | undefined, + endDate: string | undefined +): void { + if (!startDate || !endDate) return; + if (new Date(startDate) > new Date(endDate)) { + throw Errors.badRequest("invalid_range", "startDate must not be after endDate"); + } +} + +/** Whether a given entry kind is part of this request. */ +export function includesType( + query: Pick, + type: LedgerEntryType +): boolean { + return query.type === undefined || query.type.includes(type); +} + +/** A `createdAt` range, in the shape all three models share. */ +export interface CreatedAtRange { + createdAt?: { gte?: Date; lte?: Date }; +} + +/** + * The `createdAt` bound shared by all three queries, as a spreadable + * fragment. Both bounds inclusive, matching the expense and settlement lists. + * + * Typed as the structural fragment rather than one model's `WhereInput`, + * because the same object is spread into an expense, a settlement, and a + * treasury-transaction query — all three of which accept it. + * + * Returns `{}` when no range was requested, so it can always be spread into a + * `where` — and so an unfiltered ledger request produces exactly the query it + * produced before this filter existed. + */ +export function ledgerDateFilter( + query: Pick +): CreatedAtRange { + if (!query.startDate && !query.endDate) return {}; + return { + createdAt: { + ...(query.startDate && { gte: new Date(query.startDate) }), + ...(query.endDate && { lte: new Date(query.endDate) }), + }, + }; +} diff --git a/src/services/settlement-history.ts b/src/services/settlement-history.ts new file mode 100644 index 00000000..a4d5d68d --- /dev/null +++ b/src/services/settlement-history.ts @@ -0,0 +1,243 @@ +/** + * Filtering and pagination for a group's settlement history (issue #692). + * + * `GET /groups/:id/settlements` already paged, but it had no filters, so the + * only way to answer "which of our settlements failed" or "what did we settle + * in USDC last month" was to page the whole history and filter client-side — + * which is the load the pagination was there to avoid in the first place. + * + * The query construction lives here rather than in the route so the filter + * semantics can be asserted without an HTTP server, matching the split + * `src/services/expenses.ts` uses for the group expense list. Pagination + * itself is `src/lib/pagination.ts` unchanged: a `(createdAt, id)` cursor, + * `limit + 1` rows to decide `hasMore`, and the shared `meta` shape. Filters + * compose with the cursor rather than replacing it. + * + * ## Filtering by a status the database does not store + * + * The `settlements.status` column is internal and has accumulated values over + * time; clients are given the small stable set in + * `src/services/settlement-status.ts` instead. Two of those public values are + * *derived* rather than stored — a row sitting at `pending` or `submitted` + * past its `expiresAt` is publicly `expired` — so a filter that matched the + * column directly would disagree with the status the same row reports when + * read individually, which is worse than having no filter at all. + * + * `publicStatusFilter` is therefore written as the exact inverse of + * `toPublicStatus`, including its clock-skew tolerance and its treatment of + * unrecognised column values. The invariant it keeps: every row matches + * exactly one public status, so the five filtered pages partition the history + * with no row appearing twice and none unreachable. + */ +import type { Prisma } from "@prisma/client"; +import { z } from "zod"; +import { prisma } from "../db"; +import { Errors } from "../errors"; +import { + buildPage, + cursorFilter, + cursorOrderBy, + paginationQuerySchema, + requireCursor, + takeForPage, + type PageMeta, +} from "../lib/pagination"; +import { CLOCK_SKEW_TOLERANCE_SECONDS, nowSeconds } from "../lib/time-bounds"; +import { SETTLEMENT_STATUSES, type SettlementStatus } from "./settlement-status"; + +/** + * Assets a caller may filter by. + * + * `Settlement.assetCode` is a free-form column, so this is an API-level + * restriction: the filter exists to slice a history, and accepting arbitrary + * codes would let a caller probe which codes a group has used. Matches the + * expense list's `FILTERABLE_ASSETS` for the same reason. + */ +export const FILTERABLE_ASSETS = ["XLM", "USDC"] as const; + +/** + * Query parameters for the group settlement list. Extends the shared + * pagination schema; the filters are additive. + */ +export const settlementListQuerySchema = paginationQuerySchema.extend({ + status: z.enum(SETTLEMENT_STATUSES).optional(), + asset: z.enum(FILTERABLE_ASSETS).optional(), + startDate: z.string().datetime({ offset: true }).optional(), + endDate: z.string().datetime({ offset: true }).optional(), + /** + * Opt-in total count. Counting a filtered set is a second query over every + * matching row, which the pagination contract deliberately avoids paying on + * every request — so clients that need a total ask for it explicitly. + */ + includeTotal: z + .union([z.boolean(), z.enum(["true", "false"])]) + .transform((value) => value === true || value === "true") + .optional(), +}); + +export type SettlementListQuery = z.infer; + +export interface SettlementListPage { + items: T[]; + meta: PageMeta & { total?: number }; +} + +/** Column values that mean what they say, whatever the clock reads. */ +const TERMINAL_COLUMN_STATUSES = ["confirmed", "failed", "expired"] as const; + +/** + * The instant before which a recorded expiry counts as lapsed. + * + * Derived the same way `isIntentExpired` derives it — whole seconds, minus the + * shared skew tolerance — so a row cannot be `expired` to this filter and + * `submitted` to a direct read of the same settlement a millisecond later. + */ +export function expiryCutoff(now: Date = new Date()): Date { + return new Date((nowSeconds(now) - CLOCK_SKEW_TOLERANCE_SECONDS) * 1000); +} + +/** + * Translate a public status into a condition on the stored columns. + * + * The inverse of `toPublicStatus`: + * + * - `confirmed` / `failed` — the column says so; expiry is irrelevant. + * - `expired` — the column says so, *or* a row still in flight + * (`pending`/`submitted`, or any unrecognised value) whose + * recorded expiry has lapsed. + * - `submitted` — the column says `submitted` and the expiry has not lapsed + * (a null expiry is a row predating expiry tracking, which is + * never treated as expired). + * - `awaiting_signature` — anything not in the four cases above whose expiry + * has not lapsed. Expressed as `notIn` rather than + * `equals: "pending"` so an unrecognised column value lands + * here, exactly as `toPublicStatus`'s `default` branch + * reports it: not yet signed, definitely not paid. + */ +export function publicStatusFilter( + status: SettlementStatus, + now: Date = new Date() +): Prisma.SettlementWhereInput { + const cutoff = expiryCutoff(now); + const notLapsed: Prisma.SettlementWhereInput = { + OR: [{ expiresAt: null }, { expiresAt: { gte: cutoff } }], + }; + + switch (status) { + case "confirmed": + return { status: "confirmed" }; + case "failed": + return { status: "failed" }; + case "expired": + return { + OR: [ + { status: "expired" }, + { + status: { notIn: [...TERMINAL_COLUMN_STATUSES] }, + expiresAt: { lt: cutoff }, + }, + ], + }; + case "submitted": + return { AND: [{ status: "submitted" }, notLapsed] }; + case "awaiting_signature": + return { + AND: [{ status: { notIn: [...TERMINAL_COLUMN_STATUSES, "submitted"] } }, notLapsed], + }; + } +} + +/** + * Reject a reversed range up front. Without this the query is valid but can + * only ever return nothing, which reads to a client as "no settlements" + * rather than "your dates are backwards". + */ +export function assertValidRange( + startDate: string | undefined, + endDate: string | undefined +): void { + if (!startDate || !endDate) return; + if (new Date(startDate) > new Date(endDate)) { + throw Errors.badRequest("invalid_range", "startDate must not be after endDate"); + } +} + +/** Both bounds inclusive, matching the expense list and the audit log. */ +function dateFilter( + startDate: string | undefined, + endDate: string | undefined +): Prisma.SettlementWhereInput { + return { + createdAt: { + ...(startDate && { gte: new Date(startDate) }), + ...(endDate && { lte: new Date(endDate) }), + }, + }; +} + +/** + * The filter conditions for a query, excluding pagination. + * + * Exported so the semantics can be asserted against the shape handed to + * Prisma, without standing up a route. + */ +export function settlementFilters( + groupId: string, + query: Pick, + now: Date = new Date() +): Prisma.SettlementWhereInput[] { + return [ + { groupId }, + ...(query.asset ? [{ assetCode: query.asset }] : []), + ...(query.status ? [publicStatusFilter(query.status, now)] : []), + ...(query.startDate || query.endDate + ? [dateFilter(query.startDate, query.endDate)] + : []), + ]; +} + +/** + * List one page of a group's settlements. + * + * The caller must have authorized group access first: this function scopes by + * `groupId` but performs no membership check of its own. + */ +export async function listGroupSettlements( + groupId: string, + query: SettlementListQuery, + include: Prisma.SettlementInclude, + now: Date = new Date() +): Promise> { + assertValidRange(query.startDate, query.endDate); + + const position = requireCursor(query.cursor); + const filters = settlementFilters(groupId, query, now); + const cursorCondition = cursorFilter(position, query.order); + + // Unfiltered requests keep the flat `{ groupId, ...cursor }` shape the route + // has always used; only a filtered one pays for the `AND` wrapper. + const where: Prisma.SettlementWhereInput = + filters.length === 1 + ? { groupId, ...cursorCondition } + : { AND: [...filters, cursorCondition] }; + + const rows = (await prisma.settlement.findMany({ + where, + include, + orderBy: cursorOrderBy(query.order), + take: takeForPage(query.limit), + })) as unknown as T[]; + + const { items, meta } = buildPage(rows, query.limit, query.order); + + if (!query.includeTotal) { + return { items, meta }; + } + + // Counted over the filters alone — the cursor describes where this page + // starts, so including it would count only the remainder of the scan. + const total = await prisma.settlement.count({ + where: filters.length === 1 ? { groupId } : { AND: filters }, + }); + return { items, meta: { ...meta, total } }; +} diff --git a/src/services/stellar.ts b/src/services/stellar.ts index 8e00945f..43eb9b6e 100644 --- a/src/services/stellar.ts +++ b/src/services/stellar.ts @@ -663,29 +663,44 @@ export function verifySignedPaymentXdr( } /** - * True only if at least one signature on the envelope verifies against the - * transaction's own source account, hashed with our configured network - * passphrase. An empty signature list (unsigned) or a signature produced - * against a different network passphrase both fail this check. + * True only if at least one signature on the envelope verifies against + * `publicKey`, over the transaction hash — which is computed from bytes that + * include the network passphrase, so a signature produced for another network + * fails here too. An unsigned envelope, and a `publicKey` that is not a valid + * ed25519 account, both answer false rather than throwing: the caller's job is + * to reject, and an unparseable key is not a signature it can accept. + * + * Exported because this is the only signature check in the codebase — the + * submission paths, the multisig collector, and the non-throwing inspector in + * src/services/xdr-inspection.ts all route through it. */ -function hasValidSourceSignature(tx: Transaction): boolean { +export function verifySourceSignature(tx: Transaction, publicKey: string): boolean { if (!tx.signatures || tx.signatures.length === 0) return false; - let sourceKeypair: Keypair; + let keypair: Keypair; try { - sourceKeypair = Keypair.fromPublicKey(tx.source); + keypair = Keypair.fromPublicKey(publicKey); } catch { return false; } const hash = tx.hash(); return tx.signatures.some((sig) => { try { - return sourceKeypair.verify(hash, sig.signature()); + return keypair.verify(hash, sig.signature()); } catch { return false; } }); } +/** + * {@link verifySourceSignature} against the envelope's *own* declared source, + * for the paths that check authorship before they know which account the + * intent names. + */ +function hasValidSourceSignature(tx: Transaction): boolean { + return verifySourceSignature(tx, tx.source); +} + /** * What a signed envelope must match to be the payment this API authorized. * Every field a wallet could alter between XDR creation and submission is @@ -698,7 +713,17 @@ export interface PaymentExpectation { asset: AssetSpec; amount: string; memoCode: string; - /** Sequence used when the server created the unsigned intent. */ + /** + * The envelope's **own** sequence number, as `tx.sequence` reports it. + * + * Note the off-by-one against `stellar.buildPayment`, whose `sourceSequence` + * argument is the *account* sequence Horizon reported: a transaction built + * from account sequence `n` carries sequence `n + 1`. An intent that records + * the account sequence must add one before putting it here, or the check + * below rejects every envelope it was built for. Left unset — as every + * caller leaves it today, there being no persisted column — the sequence is + * not checked. + */ sourceSequence?: string; /** Maximum fee per operation accepted for this intent. */ maxFeeStroops?: number; @@ -766,23 +791,66 @@ export function assertTimeBoundsValid( } /** - * Structural check that a transaction is exactly the payment we authorized. + * Which part of the envelope diverged from the intent. + * + * The name is stable and machine-readable, so a caller that needs to react to + * *what* diverged (an inspector reporting a field, a log line grouping + * rejections by cause) does not have to pattern-match on prose. One name per + * check in {@link matchPaymentIntent}. + */ +export type PaymentIntentField = + | "source" + | "sequence" + | "operationCount" + | "fee" + | "operationType" + | "operationSource" + | "destination" + | "assetIssuer" + | "asset" + | "amount" + | "memo"; + +/** A single divergence between an envelope and the intent it should match. */ +export interface PaymentIntentMismatch { + field: PaymentIntentField; + message: string; +} + +/** + * Structural check that a transaction is exactly the payment we authorized, + * reported rather than thrown. * * Ordered deliberately: shape before content, so an envelope carrying extra * operations is reported as such rather than as an incidental fee or asset - * mismatch further down. + * mismatch further down. Returns the *first* divergence, or null when the + * envelope matches — signatures are not its concern (see + * {@link verifySourceSignature}). + * + * This is the single description of what "matches the intent" means. + * `assertMatchesIntent` below throws it as an `AppError` for the submission + * paths; `src/services/xdr-inspection.ts` reports it field-by-field for + * callers that want to inspect an envelope without raising. Neither has a + * second copy of the rules. + * + * @throws {AppError} only when `expected.asset` is not an asset this + * deployment supports — that is a fault in the *expectation* (a stored row + * or a caller's argument), not in the envelope being judged. */ -function assertMatchesIntent(tx: Transaction, expected: PaymentExpectation): void { +export function matchPaymentIntent( + tx: Transaction, + expected: PaymentExpectation +): PaymentIntentMismatch | null { if (tx.source !== expected.sourcePublicKey) { - throw Errors.badRequest("xdr_mismatch", "Transaction source does not match"); + return { field: "source", message: "Transaction source does not match" }; } if (expected.sourceSequence !== undefined && tx.sequence.toString() !== expected.sourceSequence) { - throw Errors.badRequest("xdr_mismatch", "Transaction sequence does not match"); + return { field: "sequence", message: "Transaction sequence does not match" }; } if (tx.operations.length !== 1) { - throw Errors.badRequest("xdr_mismatch", "Expected exactly one operation"); + return { field: "operationCount", message: "Expected exactly one operation" }; } const fee = Number(tx.fee); @@ -792,21 +860,21 @@ function assertMatchesIntent(tx: Transaction, expected: PaymentExpectation): voi fee < MIN_FEE_STROOPS_PER_OP * tx.operations.length || fee > maxFee ) { - throw Errors.badRequest( - "xdr_mismatch", - "Transaction fee is outside the accepted range" - ); + return { field: "fee", message: "Transaction fee is outside the accepted range" }; } const op = tx.operations[0] as any; if (op.type !== "payment") { - throw Errors.badRequest("xdr_mismatch", "Expected a payment operation"); + return { field: "operationType", message: "Expected a payment operation" }; } if (op.source && op.source !== expected.sourcePublicKey) { - throw Errors.badRequest("xdr_mismatch", "Payment operation source does not match"); + return { + field: "operationSource", + message: "Payment operation source does not match", + }; } if (op.destination !== expected.destination) { - throw Errors.badRequest("xdr_mismatch", "Payment destination does not match"); + return { field: "destination", message: "Payment destination does not match" }; } const wantAsset = toAsset(expected.asset); @@ -815,20 +883,33 @@ function assertMatchesIntent(tx: Transaction, expected: PaymentExpectation): voi // An issuer swap keeps the familiar code and changes what the recipient // actually receives, so it is called out separately from a code mismatch. if (gotAsset.getCode() === wantAsset.getCode()) { - throw Errors.badRequest("xdr_mismatch", "Payment asset issuer mismatch"); + return { field: "assetIssuer", message: "Payment asset issuer mismatch" }; } - throw Errors.badRequest("xdr_mismatch", "Payment asset does not match"); + return { field: "asset", message: "Payment asset does not match" }; } if (normalizeAmount(op.amount) !== normalizeAmount(expected.amount)) { - throw Errors.badRequest("xdr_mismatch", "Payment amount does not match"); + return { field: "amount", message: "Payment amount does not match" }; } const wantMemo = memoText(expected.memoCode); const gotMemo = tx.memo && (tx.memo as any).value ? (tx.memo as any).value.toString() : ""; if (gotMemo !== wantMemo) { - throw Errors.badRequest("xdr_mismatch", "Memo does not match the expense reference"); + return { field: "memo", message: "Memo does not match the expense reference" }; + } + + return null; +} + +/** + * {@link matchPaymentIntent} as an assertion: the submission paths want the + * first divergence to abort the request, with the stable `xdr_mismatch` code. + */ +function assertMatchesIntent(tx: Transaction, expected: PaymentExpectation): void { + const mismatch = matchPaymentIntent(tx, expected); + if (mismatch) { + throw Errors.badRequest("xdr_mismatch", mismatch.message); } } @@ -877,16 +958,7 @@ export function validatePaymentTx(tx: Transaction, expected: PaymentExpectation) // Skipped for multisig accounts, whose own key never signs — verifyMultisig // checks authorization there instead. if (!expected.skipSourceSignatureCheck) { - const sourceKeypair = Keypair.fromPublicKey(expected.sourcePublicKey); - const txHash = tx.hash(); - const hasValidSignature = tx.signatures.some((sig) => { - try { - return sourceKeypair.verify(txHash, sig.signature()); - } catch { - return false; - } - }); - if (!hasValidSignature) { + if (!verifySourceSignature(tx, expected.sourcePublicKey)) { throw Errors.badRequest( "xdr_mismatch", "Transaction signature is invalid or for the wrong network" diff --git a/src/services/webhook-listing.ts b/src/services/webhook-listing.ts new file mode 100644 index 00000000..d555b46f --- /dev/null +++ b/src/services/webhook-listing.ts @@ -0,0 +1,192 @@ +/** + * Paginated reads of a group's webhook endpoints and their delivery attempts + * (issue #692). + * + * ## Why these two lists needed their own module + * + * Both routes used to answer with an unbounded `findMany`. For the endpoint + * list that is merely untidy — a group has a handful of endpoints. For + * deliveries it is a real liability: one row is written per event per + * endpoint, with a row kept for every retry, so an active group accumulates + * them without limit. `GET /groups/:groupId/webhooks/:webhookId/deliveries` + * therefore grew without bound in three places at once — the database scan, + * the JSON serialization, and the response body — and the first request a + * busy group made after a quiet week was the one that noticed. + * + * Paging both of them is the fix. The query construction lives here rather + * than in the route so the filter semantics can be tested without an HTTP + * server, which is the same split `src/services/expenses.ts` uses for the + * group expense list. + * + * ## Contract + * + * `src/lib/pagination.ts`, unchanged: a `(createdAt, id)` cursor, `limit + 1` + * rows fetched to decide `hasMore` without a second count query, and the one + * `meta` shape every list endpoint in this API returns. Filters compose with + * the cursor rather than replacing it, so a filtered scan pages exactly like + * an unfiltered one. + * + * ## What a delivery row may disclose + * + * A delivery carries the exact bytes that were signed and sent (`payload`) and + * whatever the receiver answered (`responseBody`). Neither belongs in a list + * response: the payload is the group's own data re-served through a different + * authorization path, and the response body is text from a third-party server + * echoed back to a group member. `deliverySummary` is the allowlist of fields + * a list may return, so adding a column to the model does not silently widen + * what this endpoint exposes. + */ +import type { Prisma } from "@prisma/client"; +import { z } from "zod"; +import { prisma } from "../db"; +import { + buildPage, + cursorFilter, + cursorOrderBy, + paginationQuerySchema, + requireCursor, + takeForPage, + type PageMeta, +} from "../lib/pagination"; +import { WEBHOOK_EVENT_TYPES } from "./event"; + +/** Delivery states the worker records, as clients may filter by them. */ +export const DELIVERY_STATUSES = ["pending", "delivered", "failed"] as const; +export type DeliveryStatus = (typeof DELIVERY_STATUSES)[number]; + +/** + * Query parameters for the delivery list. + * + * Extends the shared pagination schema, so cursor/limit/order behave exactly + * as they do on every other paginated route; the two filters are additive. + * `status` answers the question the endpoint exists for — "what is failing to + * reach my endpoint" — without making the caller page through the successes. + */ +export const deliveryListQuerySchema = paginationQuerySchema.extend({ + status: z.enum(DELIVERY_STATUSES).optional(), + event: z.enum(WEBHOOK_EVENT_TYPES).optional(), +}); + +export type DeliveryListQuery = z.infer; + +/** Query parameters for the endpoint list: pagination only, no filters. */ +export const webhookListQuerySchema = paginationQuerySchema; + +export interface ListPage { + items: T[]; + meta: PageMeta; +} + +/** + * The fields a delivery list returns. Deliberately an allowlist — see the + * module comment on `payload` and `responseBody`. + */ +export interface DeliverySummary { + id: string; + webhookId: string; + eventType: string; + status: string; + attempts: number; + responseStatusCode: number | null; + nextAttemptAt: string | null; + lastAttemptAt: string | null; + createdAt: string; + updatedAt: string; +} + +export function deliverySummary(row: { + id: string; + webhookId: string; + eventType: string; + status: string; + attempts: number; + responseStatusCode: number | null; + nextAttemptAt: Date | null; + lastAttemptAt: Date | null; + createdAt: Date; + updatedAt: Date; +}): DeliverySummary { + return { + id: row.id, + webhookId: row.webhookId, + eventType: row.eventType, + status: row.status, + attempts: row.attempts, + responseStatusCode: row.responseStatusCode ?? null, + nextAttemptAt: row.nextAttemptAt ? row.nextAttemptAt.toISOString() : null, + lastAttemptAt: row.lastAttemptAt ? row.lastAttemptAt.toISOString() : null, + createdAt: row.createdAt.toISOString(), + updatedAt: row.updatedAt.toISOString(), + }; +} + +/** + * The filter conditions for a delivery query, excluding pagination. + * + * Exported so the semantics can be asserted against the shape handed to + * Prisma, without standing up a route. + */ +export function deliveryFilters( + webhookId: string, + query: Pick +): Prisma.WebhookDeliveryWhereInput[] { + return [ + { webhookId }, + ...(query.status ? [{ status: query.status }] : []), + ...(query.event ? [{ eventType: query.event }] : []), + ]; +} + +/** + * List one page of a webhook's delivery attempts. + * + * The caller must have authorized access to the endpoint's group first: this + * function scopes by `webhookId` and performs no membership check of its own. + */ +export async function listWebhookDeliveries( + webhookId: string, + query: DeliveryListQuery +): Promise> { + const position = requireCursor(query.cursor); + const filters = deliveryFilters(webhookId, query); + const cursorCondition = cursorFilter(position, query.order); + + // With no filters beyond the endpoint scope the query keeps its flat shape; + // only a filtered request pays for the `AND` wrapper. Either way the cursor + // condition stays separate from the filters, so paging never widens the + // result set. + const where: Prisma.WebhookDeliveryWhereInput = + filters.length === 1 + ? { webhookId, ...cursorCondition } + : { AND: [...filters, cursorCondition] }; + + const rows = await prisma.webhookDelivery.findMany({ + where, + orderBy: cursorOrderBy(query.order), + take: takeForPage(query.limit), + }); + + const { items, meta } = buildPage(rows, query.limit, query.order); + return { items: items.map(deliverySummary), meta }; +} + +/** + * List one page of a group's webhook endpoints. + * + * As above, group access is the caller's responsibility; `groupId` is what + * scopes the page, never the cursor. + */ +export async function listGroupWebhooks( + groupId: string, + query: z.infer +): Promise> { + const position = requireCursor(query.cursor); + + const rows = await prisma.webhook.findMany({ + where: { groupId, ...cursorFilter(position, query.order) }, + orderBy: cursorOrderBy(query.order), + take: takeForPage(query.limit), + }); + + return buildPage(rows, query.limit, query.order); +} diff --git a/src/services/xdr-inspection.ts b/src/services/xdr-inspection.ts new file mode 100644 index 00000000..b557ddf2 --- /dev/null +++ b/src/services/xdr-inspection.ts @@ -0,0 +1,247 @@ +/** + * Inspect a wallet-signed Stellar envelope against the intent it must match, + * and report what diverged instead of throwing (issue #694). + * + * ## Why this exists alongside `validateSignedXdr` + * + * Mergepay never holds a private key. It builds an unsigned XDR, hands it to a + * wallet, and gets *something* back — a compromised or buggy wallet can return + * a validly signed transaction that pays a different account, a different + * asset, or a different amount, and Horizon would accept it happily. The + * submission paths guard against that with `validateSignedXdr`, which throws + * the first divergence as a 400 so the request stops. + * + * Some callers need the same judgement without the throw: an operator tool + * explaining why an envelope was refused, a reconciliation pass classifying a + * stored envelope, a validator that wants the field name rather than prose. + * Writing a second validator for them is how a weaker one ends up in + * production — it was: `src/utils/stellar-xdr.ts` carried a copy that never + * checked signatures or sequence numbers, compared amounts as raw strings (so + * `"10"` did not match `"10.0000000"`), and reported a fee-bump envelope as + * "must contain exactly one operation". + * + * So this module adds no rules of its own. It composes the same three + * primitives the submission paths use: + * + * 1. `parseSignedPaymentXdr` — decode, and reject fee-bump wrappers; + * 2. `assertTimeBoundsValid` — the envelope's validity window against the + * intent's recorded expiry; + * 3. `matchPaymentIntent` — source, sequence, operation count, fee band, + * operation type and source, destination, asset code and issuer, amount, + * and memo; + * 4. `verifySourceSignature` — a signature that verifies against the + * expected source account for the configured network passphrase. + * + * ## Order + * + * Shape before content, content before signature. An envelope that is not the + * payment we authorized is reported as *that*, not as a signature problem — + * which is the more useful answer, since a wallet returning the wrong payment + * will have signed it perfectly. + * + * ## What a rejection may say + * + * Only which field diverged, never the value found. The envelope is untrusted + * input and a rejection is logged and returned to a client, so echoing its + * contents back would both leak the caller's own payload into logs and turn + * this into an oracle for probing stored intents. + */ +import { AppError } from "../lib/errors"; +import { + assertTimeBoundsValid, + matchPaymentIntent, + parseSignedPaymentXdr, + verifySourceSignature, + type PaymentExpectation, + type PaymentIntentField, +} from "./stellar"; +import type { Transaction } from "@stellar/stellar-sdk"; + +/** + * Why an envelope was refused. + * + * - `xdr_malformed` — it did not decode as a transaction envelope at all; + * - `xdr_mismatch` — it decoded, but is not the payment that was authorized + * (including a fee-bump wrapper, which is a different + * transaction than the one we built); + * - `intent_expired` — it decoded and matched, but its validity window has + * lapsed, or the intent behind it has; + * - `xdr_unsigned` — it matched the intent but carries no signature from the + * source account for this network. + * + * The first three are the codes the submission paths already raise, so a + * caller that handles one handles both layers. + */ +export type XdrRejectionCode = + | "xdr_malformed" + | "xdr_mismatch" + | "intent_expired" + | "xdr_unsigned"; + +/** Which part of the envelope was refused. */ +export type XdrRejectionField = + | PaymentIntentField + | "envelope" + | "timeBounds" + | "signature"; + +export interface XdrInspectionRejected { + ok: false; + code: XdrRejectionCode; + /** The part of the envelope that diverged, for callers that branch on it. */ + field: XdrRejectionField; + /** Operator- and client-safe description. Never contains envelope content. */ + message: string; +} + +export interface XdrInspectionAccepted { + ok: true; + /** The parsed envelope, for a caller that goes on to submit it. */ + tx: Transaction; + /** Its hex transaction hash — the on-chain identity of this payment. */ + hash: string; + /** Source account and sequence, as the envelope declares them. */ + source: string; + sequence: string; +} + +export type XdrInspection = XdrInspectionAccepted | XdrInspectionRejected; + +export interface InspectOptions { + /** + * Require a signature from the expected source account. On by default: an + * unsigned envelope is not an authorization, and accepting one is how a + * "validated" envelope reaches a submission path on nobody's authority. + * + * Turn it off only where authorship is established elsewhere — an unsigned + * intent being read back for display, or a multisig proposal whose shared + * source account never signs with its own key (`verifyMultisig` checks + * authorization there instead). + */ + requireSignature?: boolean; +} + +/** + * Inspect `signedXdr` against `expected` and report the first divergence. + * + * @param signedXdr - The base64 transaction envelope as the wallet returned it. + * @param expected - The server-issued intent, rebuilt from stored state — + * never from anything the client sent in the same request. + * @param options - See {@link InspectOptions}. + * @returns `{ ok: true, tx, hash, source, sequence }` when the envelope is + * exactly the authorized payment, otherwise `{ ok: false, code, field, + * message }`. + */ +export function inspectSignedTransaction( + signedXdr: string, + expected: PaymentExpectation, + options: InspectOptions = {} +): XdrInspection { + const requireSignature = options.requireSignature ?? true; + + let tx: Transaction; + try { + tx = parseSignedPaymentXdr(signedXdr, "Signed transaction could not be parsed"); + } catch (error) { + // `parseSignedPaymentXdr` raises `xdr_malformed` for undecodable input and + // `xdr_mismatch` for a fee-bump wrapper; both arrive here as an AppError + // whose code is already the one this result reports. + return fromAppError(error, "envelope"); + } + + try { + assertTimeBoundsValid(tx, expected.expiresAt, expected.resource); + } catch (error) { + return fromAppError(error, "timeBounds"); + } + + let mismatch; + try { + mismatch = matchPaymentIntent(tx, expected); + } catch (error) { + // Only reachable when the *expectation* names an asset this deployment + // does not support, which is a fault in the stored intent rather than in + // the envelope. Reported rather than thrown so one bad row cannot crash a + // caller that is inspecting a batch. + return fromAppError(error, "asset"); + } + if (mismatch) { + return { + ok: false, + code: "xdr_mismatch", + field: mismatch.field, + message: mismatch.message, + }; + } + + if (requireSignature && !verifySourceSignature(tx, expected.sourcePublicKey)) { + return { + ok: false, + code: "xdr_unsigned", + field: "signature", + message: + "Transaction is not validly signed by its source account for the configured network", + }; + } + + return { + ok: true, + tx, + hash: tx.hash().toString("hex"), + source: tx.source, + sequence: tx.sequence.toString(), + }; +} + +/** + * {@link inspectSignedTransaction} as an assertion, for a route that wants the + * standard 400 envelope rather than a result to branch on. + * + * The thrown error carries the rejection's `field` in its details, so a client + * can tell "you changed the amount" from "you changed the destination" without + * parsing the message. + */ +export function assertSignedTransactionMatchesIntent( + signedXdr: string, + expected: PaymentExpectation, + options: InspectOptions = {} +): XdrInspectionAccepted { + const result = inspectSignedTransaction(signedXdr, expected, options); + if (!result.ok) { + throw new AppError(400, result.code.toUpperCase(), result.message, { + field: result.field, + }); + } + return result; +} + +/** + * Re-shape an `AppError` raised by one of the shared primitives into a + * rejection, preserving its code so both layers speak the same vocabulary. + * + * A non-`AppError` would mean a primitive failed for a reason this module has + * no account of; it is re-thrown rather than flattened into a "mismatch", + * because reporting an internal fault as a client error is how a real defect + * gets mistaken for a bad request. + */ +function fromAppError(error: unknown, field: XdrRejectionField): XdrInspectionRejected { + if (!(error instanceof AppError)) { + throw error; + } + const code = error.code.toLowerCase(); + return { + ok: false, + code: isRejectionCode(code) ? code : "xdr_mismatch", + field, + message: error.message, + }; +} + +function isRejectionCode(code: string): code is XdrRejectionCode { + return ( + code === "xdr_malformed" || + code === "xdr_mismatch" || + code === "intent_expired" || + code === "xdr_unsigned" + ); +} diff --git a/src/utils/__tests__/stellar-xdr.test.ts b/src/utils/__tests__/stellar-xdr.test.ts index 1f871afe..e44298d4 100644 --- a/src/utils/__tests__/stellar-xdr.test.ts +++ b/src/utils/__tests__/stellar-xdr.test.ts @@ -6,19 +6,23 @@ * valid (signed and unsigned) envelopes, input that is not base64 at all, * base64 that does not decode to an XDR, and envelopes that were corrupted * or truncated in transit. + * + * Intent validation is deliberately absent here: it belongs to + * `src/services/xdr-inspection.ts` and is covered by + * tests/services/xdr-inspection.test.ts. This module's own intent validator + * was removed in issue #694 because it was a weaker second copy of those + * rules. */ import { describe, it, expect } from "vitest"; import { Account, - Asset, BASE_FEE, Keypair, - Memo, Networks, Operation, TransactionBuilder, } from "@stellar/stellar-sdk"; -import { isValidXdr, validateTransactionIntent } from "../stellar-xdr"; +import { isValidXdr } from "../stellar-xdr"; /** Build a valid, unsigned transaction envelope for the test network. */ function buildXdr(): string { @@ -121,122 +125,3 @@ describe("isValidXdr", () => { expect(isValidXdr(buildXdr())).toBe(true); }); }); - -describe("validateTransactionIntent", () => { - it("accepts a valid payment XDR matching the expectation", () => { - const payer = Keypair.random(); - const recipient = Keypair.random(); - const tx = new TransactionBuilder( - new Account(payer.publicKey(), "100"), - { fee: BASE_FEE, networkPassphrase: Networks.TESTNET } - ) - .addOperation( - Operation.payment({ - destination: recipient.publicKey(), - asset: Asset.native(), - amount: "50.0000000", - }) - ) - .addMemo(Memo.text("EXP123")) - .setTimeout(300) - .build(); - - const result = validateTransactionIntent(tx.toXDR(), { - sourcePublicKey: payer.publicKey(), - destination: recipient.publicKey(), - assetCode: "XLM", - amount: "50.0000000", - memo: "EXP123", - }, Networks.TESTNET); - - expect(result.valid).toBe(true); - expect(result.error).toBeUndefined(); - }); - - it("rejects XDR with an incorrect destination account", () => { - const payer = Keypair.random(); - const recipient = Keypair.random(); - const wrongRecipient = Keypair.random(); - const tx = new TransactionBuilder( - new Account(payer.publicKey(), "100"), - { fee: BASE_FEE, networkPassphrase: Networks.TESTNET } - ) - .addOperation( - Operation.payment({ - destination: wrongRecipient.publicKey(), - asset: Asset.native(), - amount: "50.0000000", - }) - ) - .setTimeout(300) - .build(); - - const result = validateTransactionIntent(tx.toXDR(), { - sourcePublicKey: payer.publicKey(), - destination: recipient.publicKey(), - assetCode: "XLM", - amount: "50.0000000", - }, Networks.TESTNET); - - expect(result.valid).toBe(false); - expect(result.error).toContain("destination mismatch"); - }); - - it("rejects XDR with an altered payment amount", () => { - const payer = Keypair.random(); - const recipient = Keypair.random(); - const tx = new TransactionBuilder( - new Account(payer.publicKey(), "100"), - { fee: BASE_FEE, networkPassphrase: Networks.TESTNET } - ) - .addOperation( - Operation.payment({ - destination: recipient.publicKey(), - asset: Asset.native(), - amount: "999.0000000", - }) - ) - .setTimeout(300) - .build(); - - const result = validateTransactionIntent(tx.toXDR(), { - sourcePublicKey: payer.publicKey(), - destination: recipient.publicKey(), - assetCode: "XLM", - amount: "50.0000000", - }, Networks.TESTNET); - - expect(result.valid).toBe(false); - expect(result.error).toContain("amount mismatch"); - }); - - it("rejects XDR with a memo mismatch", () => { - const payer = Keypair.random(); - const recipient = Keypair.random(); - const tx = new TransactionBuilder( - new Account(payer.publicKey(), "100"), - { fee: BASE_FEE, networkPassphrase: Networks.TESTNET } - ) - .addOperation( - Operation.payment({ - destination: recipient.publicKey(), - asset: Asset.native(), - amount: "50.0000000", - }) - ) - .addMemo(Memo.text("WRONG")) - .setTimeout(300) - .build(); - - const result = validateTransactionIntent(tx.toXDR(), { - sourcePublicKey: payer.publicKey(), - destination: recipient.publicKey(), - assetCode: "XLM", - amount: "50.0000000", - memo: "EXP123", - }, Networks.TESTNET); - - expect(result.valid).toBe(false); - expect(result.error).toContain("memo mismatch"); - }); -}); diff --git a/src/utils/stellar-xdr.ts b/src/utils/stellar-xdr.ts index 467d9abd..d9a17829 100644 --- a/src/utils/stellar-xdr.ts +++ b/src/utils/stellar-xdr.ts @@ -20,9 +20,14 @@ import { config } from "../config"; * - truncated or otherwise corrupted envelopes. * * The check is parse-level: it confirms the envelope decodes for the given - * network passphrase but does not verify signatures or transaction intent — - * that deeper validation lives with the handlers that need it (see - * `validateSignedXdr` in src/services/stellar.ts). + * network passphrase but does not verify signatures or transaction intent. + * That deeper validation has exactly one home — `src/services/stellar.ts` for + * the submission paths that throw, and `src/services/xdr-inspection.ts` for + * callers that want the diverging field reported instead (issue #694). A + * second intent validator used to live in this module and was weaker than + * both: it never checked signatures or sequence numbers, compared amounts as + * raw strings, and mis-reported fee-bump envelopes. Add nothing of that kind + * here. * * @param xdr - Candidate base64 XDR string. * @param networkPassphrase - Defaults to the configured Stellar network; the @@ -45,84 +50,3 @@ export function isValidXdr( return false; } } - -/** - * Details expected from a payment intent or expense record for XDR validation. - */ -export interface TransactionIntentExpectation { - sourcePublicKey: string; - destination: string; - assetCode: string; - assetIssuer?: string | null; - amount: string; - memo?: string; -} - -/** - * Inspect a signed transaction XDR and validate its operations against expected intent. - */ -export function validateTransactionIntent( - signedXdr: string, - expectation: TransactionIntentExpectation, - networkPassphrase: string = config.networkPassphrase -): { valid: boolean; error?: string } { - try { - const tx = TransactionBuilder.fromXDR(signedXdr, networkPassphrase); - if (!(tx instanceof TransactionBuilder.fromXDR(signedXdr, networkPassphrase).constructor)) { - // Using Transaction / FeeBumpTransaction check via instanceof - } - - const envelope = TransactionBuilder.fromXDR(signedXdr, networkPassphrase); - // @stellar/stellar-sdk Transaction has operations and source - const operations = (envelope as any).operations; - if (!operations || operations.length !== 1) { - return { valid: false, error: "Transaction must contain exactly one operation" }; - } - - const op = operations[0]; - if (op.type !== "payment") { - return { valid: false, error: "Transaction operation must be a payment" }; - } - - // Validate source account - const source = op.source || (envelope as any).source; - if (source && source !== expectation.sourcePublicKey) { - return { valid: false, error: "Transaction source account mismatch" }; - } - - // Validate destination - if (op.destination !== expectation.destination) { - return { valid: false, error: "Payment destination mismatch" }; - } - - // Validate asset - const assetCode = op.asset.isNative() ? "XLM" : op.asset.getCode(); - const assetIssuer = op.asset.isNative() ? null : op.asset.getIssuer(); - - if (assetCode !== expectation.assetCode) { - return { valid: false, error: "Payment asset code mismatch" }; - } - - if ((expectation.assetIssuer ?? null) !== (assetIssuer ?? null)) { - return { valid: false, error: "Payment asset issuer mismatch" }; - } - - // Validate amount - if (op.amount !== expectation.amount) { - return { valid: false, error: "Payment amount mismatch" }; - } - - // Validate memo if present in expectation - if (expectation.memo) { - const memoValue = (envelope as any).memo; - const memoString = memoValue && memoValue.value ? memoValue.value.toString() : ""; - if (memoString !== expectation.memo) { - return { valid: false, error: "Transaction memo mismatch" }; - } - } - - return { valid: true }; - } catch (err: any) { - return { valid: false, error: err?.message || "Failed to parse transaction XDR" }; - } -} diff --git a/tests/rate-limit-sensitive-routes.test.ts b/tests/rate-limit-sensitive-routes.test.ts index 7259f3dc..96ca4181 100644 --- a/tests/rate-limit-sensitive-routes.test.ts +++ b/tests/rate-limit-sensitive-routes.test.ts @@ -1264,3 +1264,202 @@ describe("sensitive authentication and payment endpoints rate limiting (#523)", expect(authed.headers["x-ratelimit-remaining"]).toBe("0"); }); }); + +/** + * Issues #682 / #693 — the sensitive routes that were still answering from the + * global bucket. + * + * The tiers for SEP-10 auth, settlement submission, and anchor initiation were + * already in place. What the table did not model was a second group of routes + * whose cost is not in their response: + * + * - `POST /auth/logout` revokes every refresh token a user holds and writes + * an audit row, and answers `ok` whether or not the bearer token was valid + * — so the write path is reachable anonymously; + * - `PATCH` / `DELETE /expenses/:id` rewrite what every other participant + * owes, while only expense *creation* had a budget; + * - `POST /groups/:id/treasury/enable` and `.../validate-signers` each read + * live account state from Horizon and decide who may move group funds; + * - `GET /withdraw/:id` is the anchor-transaction status read a client polls + * through an interactive flow — the one anchor route left unbudgeted; + * - `POST /groups/:groupId/webhooks/:webhookId/test` turns one inbound + * request into an outbound HTTP delivery with a retry schedule. + * + * Each case drives the real app past the policy's budget, so it fails if the + * annotation is dropped from the route declaration, and asserts the same three + * properties as the suites above: the per-route limit is what the headers + * report, the 429 carries the standard envelope, and the bucket is the + * policy's own rather than the global one. + */ +describe("sensitive routes left on the global bucket (#682, #693)", () => { + it("POST /auth/logout — per-route budget, headers, and 429 envelope", async () => { + // No Authorization header: the handler returns `{ ok: true }` immediately, + // which is exactly the path an anonymous caller can drive. + await exhaustAndAssert({ + method: "POST", + url: "/auth/logout", + max: policies.authLogout.max, + label: "auth/logout", + }); + }); + + it("POST /auth/logout is keyed by IP, so a bearer token buys no second budget", async () => { + const remoteAddress = "198.51.100.182"; + const max = policies.authLogout.max; + for (let i = 0; i < max; i++) { + await app.inject({ method: "POST", url: "/auth/logout", remoteAddress }); + } + const authed = await app.inject({ + method: "POST", + url: "/auth/logout", + remoteAddress, + headers: authHeader(), + }); + expect(authed.statusCode).toBe(429); + }); + + it("PATCH /expenses/:id — per-route budget, headers, and 429 envelope", async () => { + await exhaustAndAssert({ + method: "PATCH", + url: "/expenses/expense_rate_limit", + max: policies.expenseMutate.max, + headers: authHeader(), + payload: { title: "Edited" }, + label: "expenses/:id (patch)", + }); + }); + + it("DELETE /expenses/:id — per-route budget, headers, and 429 envelope", async () => { + // PATCH and DELETE both name `expenseMutate`, so they enforce the same + // numbers under the same per-user key. Whether they also share a *counter* + // depends on the store: `RATE_LIMIT_STORE=database` keeps one row per key, + // while the default in-memory store gives each route its own child store. + // What every deployment must hold either way is that each route enforces + // the policy, which is what this asserts. + await exhaustAndAssert({ + method: "DELETE", + url: "/expenses/expense_rate_limit_delete", + max: policies.expenseMutate.max, + headers: authHeader(), + label: "expenses/:id (delete)", + }); + }); + + it("expense edits do not spend the expense-creation budget", async () => { + // Separate key prefixes: a client correcting typos must not exhaust the + // allowance that guards opening new obligations. + const headers = authHeader(); + for (let i = 0; i < policies.expenseMutate.max + 1; i++) { + await app.inject({ + method: "PATCH", + url: "/expenses/expense_independent", + headers, + payload: { title: "Edited" }, + }); + } + const create = await app.inject({ + method: "POST", + url: "/groups/00000000-0000-0000-0000-000000000000/expenses", + headers, + payload: { + title: "Dinner", + amount: "10.0000000", + assetCode: "XLM", + splitType: "equal", + shares: [], + }, + }); + expect(create.statusCode).not.toBe(429); + }); + + it("POST /groups/:id/treasury/enable — per-route budget, headers, and 429 envelope", async () => { + await exhaustAndAssert({ + method: "POST", + url: "/groups/00000000-0000-0000-0000-000000000000/treasury/enable", + max: policies.treasuryConfig.max, + headers: authHeader(), + payload: { publicKey: Keypair.random().publicKey() }, + label: "treasury/enable", + }); + }); + + it("POST /groups/:id/treasury/validate-signers — per-route budget and 429 envelope", async () => { + await exhaustAndAssert({ + method: "POST", + url: "/groups/00000000-0000-0000-0000-000000000000/treasury/validate-signers", + max: policies.treasuryConfig.max, + headers: authHeader(), + payload: {}, + label: "treasury/validate-signers", + }); + }); + + it("treasury configuration does not spend the budget that guards moving funds", async () => { + // `treasuryConfig` has its own key prefix, so exhausting it leaves + // `treasuryPropose` — the budget for actually moving money — intact. + const headers = authHeader(); + const enableUrl = "/groups/00000000-0000-0000-0000-000000000000/treasury/enable"; + for (let i = 0; i < policies.treasuryConfig.max + 1; i++) { + await app.inject({ + method: "POST", + url: enableUrl, + headers, + payload: { publicKey: Keypair.random().publicKey() }, + }); + } + const propose = await app.inject({ + method: "POST", + url: "/treasury/proposals", + headers, + payload: {}, + }); + expect(propose.statusCode).not.toBe(429); + }); + + it("GET /withdraw/:id — takes the anchor-poll budget, not the global one", async () => { + await exhaustAndAssert({ + method: "GET", + url: "/withdraw/wth_rate_limit_read", + max: policies.anchorPoll.max, + headers: authHeader(), + label: "withdraw/:id (read)", + }); + }); + + it("POST /groups/:groupId/webhooks/:webhookId/test — per-route budget, headers, and 429 envelope", async () => { + await exhaustAndAssert({ + method: "POST", + url: "/groups/00000000-0000-0000-0000-000000000000/webhooks/wh_rate_limit/test", + max: policies.webhookTest.max, + headers: authHeader(), + label: "webhooks/:id/test", + }); + }); + + it("firing a test delivery does not spend the webhook-registration budget", async () => { + const headers = authHeader(); + const base = "/groups/00000000-0000-0000-0000-000000000000/webhooks"; + for (let i = 0; i < policies.webhookTest.max + 1; i++) { + await app.inject({ method: "POST", url: `${base}/wh_independent/test`, headers }); + } + const register = await app.inject({ + method: "POST", + url: base, + headers, + payload: { url: "https://example.test/hook", events: ["expense.created"] }, + }); + expect(register.statusCode).not.toBe(429); + }); + + it("every newly budgeted route is tighter than the global default", async () => { + for (const name of [ + "authLogout", + "expenseMutate", + "treasuryConfig", + "webhookTest", + ] as const) { + expect(policies[name].max, name).toBeLessThan(policies.global.max); + expect(policies[name].prefix, name).not.toBe(policies.global.prefix); + } + }); +}); diff --git a/tests/rate-limit-wiring.test.ts b/tests/rate-limit-wiring.test.ts index ec2c810e..f9bed00c 100644 --- a/tests/rate-limit-wiring.test.ts +++ b/tests/rate-limit-wiring.test.ts @@ -154,9 +154,18 @@ describe("sensitive routes carry the tighter per-route budget", () => { ["POST", "/api/v1/auth/challenge", "authChallenge"], ["POST", "/api/v1/auth/verify", "authVerify"], ["POST", "/api/v1/auth/refresh", "authVerify"], + // Session teardown (#682). Keyed by IP for the same reason as the two + // above: the route answers `ok` without a valid token on purpose, so + // there is no identity to key by on the path that does the work. + ["POST", "/api/v1/auth/logout", "authLogout"], // Expense creation writes a row and starts a payment split, so it is // budgeted on its own rather than drawing on the settlement budget. ["POST", "/api/v1/groups/:id/expenses", "expenseCreate"], + // Issue #682 — the writes after creation. Amending or deleting an expense + // rewrites every other participant's obligations, and was on the global + // bucket while creation had its own. + ["PATCH", "/api/v1/expenses/:id", "expenseMutate"], + ["DELETE", "/api/v1/expenses/:id", "expenseMutate"], // Settlement submission and confirmation. ["POST", "/api/v1/expenses/:id/settle", "settlementCreate"], ["POST", "/api/v1/groups/:id/settlements", "settlementCreate"], @@ -179,6 +188,10 @@ describe("sensitive routes carry the tighter per-route budget", () => { // Issue #657's collection-style aliases of the same two operations. ["POST", "/api/v1/treasury/propose", "treasuryPropose"], ["POST", "/api/v1/treasury/:id/sign", "treasurySubmit"], + // Issue #682 — deciding *who* may move group funds, as opposed to moving + // it. Both routes read live account state from Horizon per call. + ["POST", "/api/v1/groups/:id/treasury/enable", "treasuryConfig"], + ["POST", "/api/v1/groups/:id/treasury/validate-signers", "treasuryConfig"], // Anchor / SEP-24. Initiation fans out upstream, so it is the tightest. ["POST", "/api/v1/anchors/deposit", "anchorInit"], ["POST", "/api/v1/anchors/withdraw", "anchorInit"], @@ -194,6 +207,11 @@ describe("sensitive routes carry the tighter per-route budget", () => { // The list form of the same poll (#655): one page of the same DB-only // status reads, so it takes the same budget rather than a new one. ["GET", "/api/v1/sep24/transactions", "anchorPoll"], + // Issue #693 — the withdrawal status read a client polls through an + // interactive anchor flow. The same DB-only status read as the session + // polls above, and it was the one anchor-transaction route left on the + // global bucket. + ["GET", "/api/v1/withdraw/:id", "anchorPoll"], ["POST", "/api/v1/anchors/webhook", "anchorWebhook"], ["POST", "/api/v1/sep24/callback", "sep24Callback"], ["POST", "/api/v1/webhooks/sep24", "sep24Webhook"], @@ -208,6 +226,10 @@ describe("sensitive routes carry the tighter per-route budget", () => { ["POST", "/api/v1/groups/join", "groupJoin"], ["POST", "/api/v1/webhooks", "webhookCreate"], ["POST", "/api/v1/groups/:groupId/webhooks", "webhookCreate"], + // Issue #693 — ringing a registered endpoint. `webhookCreate` bounded + // registering one; this is the cheaper half of the same amplification, so + // it takes the tightest budget in the table. + ["POST", "/api/v1/groups/:groupId/webhooks/:webhookId/test", "webhookTest"], ["POST", "/api/v1/uploads/receipt", "uploadReceipt"], ]; diff --git a/tests/routes/history-pagination-filtering.test.ts b/tests/routes/history-pagination-filtering.test.ts new file mode 100644 index 00000000..5b86ba17 --- /dev/null +++ b/tests/routes/history-pagination-filtering.test.ts @@ -0,0 +1,672 @@ +/** + * Issue #692 — pagination and filtering on the group transaction-history + * endpoints. + * + * The shared cursor helpers are unit-tested in tests/pagination-contract.test.ts; + * what these cases pin is route-level, because that is where the load actually + * lands: + * + * 1. **The reads are bounded.** Two of these endpoints answered with an + * unbounded `findMany` — the webhook delivery list in particular grows by + * one row per event per endpoint per retry, so the first request a busy + * group made after a quiet week was the one that noticed. Every query here + * must carry a `take` and a deterministic `orderBy`. + * 2. **Filters reach the query, not the page.** A filter applied after the + * rows are fetched would shrink pages below the requested limit and make + * `hasMore` describe the unfiltered scan. Each case asserts the shape + * handed to Prisma. + * 3. **Boundaries behave.** Empty results, a full last page, and the page + * before it return the documented `meta` rather than an error or a + * silently truncated body. + * + * Prisma is mocked throughout: the subject is the query construction and the + * response envelope, so no database is touched. + */ +import { describe, it, expect, beforeEach, vi } from "vitest"; + +const h = vi.hoisted(() => { + const model = () => ({ + create: vi.fn(), + findUnique: vi.fn(), + findFirst: vi.fn(), + findMany: vi.fn(async () => []), + count: vi.fn(async () => 0), + }); + const prisma: any = { + user: model(), + group: model(), + groupMember: model(), + expense: model(), + settlement: model(), + treasuryTransaction: model(), + webhook: model(), + webhookDelivery: model(), + }; + return { prisma }; +}); + +vi.mock("../../src/db", () => ({ prisma: h.prisma })); + +import { buildApp } from "../../src/app"; +import { signToken } from "../../src/plugins/auth"; +import { encodeCursor, MAX_PAGE_SIZE } from "../../src/lib/pagination"; +import { CLOCK_SKEW_TOLERANCE_SECONDS } from "../../src/lib/time-bounds"; +import { + expiryCutoff, + publicStatusFilter, + settlementFilters, +} from "../../src/services/settlement-history"; +import { deliveryFilters } from "../../src/services/webhook-listing"; +import { ledgerQuerySchema } from "../../src/services/ledger"; + +const prisma = h.prisma; +const USER_ID = "user_692"; +const GROUP_ID = "group_692"; +const WEBHOOK_ID = "wh_692"; + +let app: Awaited>; + +function authHeader(userId = USER_ID) { + const token = signToken({ + id: userId, + stellarPublicKey: "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", + }); + return { authorization: `Bearer ${token}` }; +} + +const AT = new Date("2026-05-01T12:00:00.000Z"); + +/** A user row complete enough for `serializeUser`, which reads `createdAt`. */ +const user = (id: string, stellarPublicKey: string) => ({ + id, + stellarPublicKey, + displayName: null, + avatarUrl: null, + createdAt: AT, +}); + +const delivery = (over: Record = {}) => ({ + id: "delivery_1", + webhookId: WEBHOOK_ID, + eventType: "expense.created", + payload: '{"secret":"the exact signed bytes"}', + status: "failed", + attempts: 3, + responseStatusCode: 500, + responseBody: "upstream said something private", + nextAttemptAt: null, + lastAttemptAt: AT, + claimedBy: null, + leaseExpiresAt: null, + createdAt: AT, + updatedAt: AT, + ...over, +}); + +const webhookRow = (over: Record = {}) => ({ + id: WEBHOOK_ID, + groupId: GROUP_ID, + userId: null, + url: "https://example.test/hook", + secret: "never-returned-on-a-list", + events: ["expense.created"], + enabled: true, + createdAt: AT, + updatedAt: AT, + ...over, +}); + +const settlementRow = (over: Record = {}) => ({ + id: "settlement_1", + shortCode: "ABC234", + groupId: GROUP_ID, + fromUserId: USER_ID, + toUserId: "user_other", + amount: "10.0000000", + assetCode: "USDC", + assetIssuer: null, + stellarTxHash: null, + transactionXdr: null, + status: "pending", + expiresAt: null, + createdAt: AT, + updatedAt: AT, + retryCount: 0, + from: user(USER_ID, "GA"), + to: user("user_other", "GB"), + ...over, +}); + +const expenseRow = (over: Record = {}) => ({ + id: "expense_1", + groupId: GROUP_ID, + payerUserId: USER_ID, + title: "Dinner", + description: null, + amount: "10.0000000", + assetCode: "XLM", + assetIssuer: null, + splitType: "equal", + memo: "ABC234", + receiptUrl: null, + createdAt: AT, + updatedAt: AT, + payer: user(USER_ID, "GA"), + shares: [], + ...over, +}); + +beforeEach(async () => { + vi.clearAllMocks(); + if (!app) app = await buildApp({ rateLimit: "off" }); + + prisma.groupMember.findUnique.mockResolvedValue({ + groupId: GROUP_ID, + userId: USER_ID, + role: "admin", + }); + prisma.group.findUnique.mockResolvedValue({ id: GROUP_ID, archived: false }); + prisma.user.findUnique.mockResolvedValue({ id: USER_ID }); + prisma.webhook.findFirst.mockResolvedValue({ id: WEBHOOK_ID }); + prisma.webhook.findMany.mockResolvedValue([]); + prisma.webhookDelivery.findMany.mockResolvedValue([]); + prisma.settlement.findMany.mockResolvedValue([]); + prisma.expense.findMany.mockResolvedValue([]); + prisma.treasuryTransaction.findMany.mockResolvedValue([]); +}); + +describe("GET /groups/:groupId/webhooks/:webhookId/deliveries — pagination", () => { + const url = `/groups/${GROUP_ID}/webhooks/${WEBHOOK_ID}/deliveries`; + + it("bounds the read and returns standardized page metadata", async () => { + prisma.webhookDelivery.findMany.mockResolvedValue([delivery()]); + + const res = await app.inject({ method: "GET", url, headers: authHeader() }); + + expect(res.statusCode).toBe(200); + expect(res.json().meta).toMatchObject({ + hasMore: false, + nextCursor: null, + order: "desc", + limit: 50, + }); + + const args = prisma.webhookDelivery.findMany.mock.calls[0][0]; + expect(args.take).toBeGreaterThan(0); + expect(args.orderBy).toEqual([{ createdAt: "desc" }, { id: "desc" }]); + expect(args.where.webhookId).toBe(WEBHOOK_ID); + }); + + it("returns a next cursor when a further page exists", async () => { + // limit + 1 rows: the lookahead row is what makes hasMore exact without a + // second count query. + const rows = Array.from({ length: 3 }, (_, i) => + delivery({ id: `delivery_${i}`, createdAt: new Date(2026, 4, 1, 12, 0, i) }) + ); + prisma.webhookDelivery.findMany.mockResolvedValue(rows); + + const res = await app.inject({ + method: "GET", + url: `${url}?limit=2`, + headers: authHeader(), + }); + + const body = res.json(); + expect(body.deliveries).toHaveLength(2); + expect(body.meta.hasMore).toBe(true); + expect(body.meta.nextCursor).toEqual(expect.any(String)); + }); + + it("reports the last page as the last page", async () => { + // Exactly `limit` rows and no lookahead: there is nothing after this. + const rows = Array.from({ length: 2 }, (_, i) => + delivery({ id: `delivery_${i}`, createdAt: new Date(2026, 4, 1, 12, 0, i) }) + ); + prisma.webhookDelivery.findMany.mockResolvedValue(rows); + + const body = ( + await app.inject({ method: "GET", url: `${url}?limit=2`, headers: authHeader() }) + ).json(); + + expect(body.deliveries).toHaveLength(2); + expect(body.meta).toMatchObject({ hasMore: false, nextCursor: null }); + }); + + it("returns an empty page rather than an error when there are no deliveries", async () => { + const res = await app.inject({ method: "GET", url, headers: authHeader() }); + + expect(res.statusCode).toBe(200); + expect(res.json().deliveries).toEqual([]); + expect(res.json().meta).toMatchObject({ hasMore: false, nextCursor: null }); + }); + + it("resumes strictly after the cursor position", async () => { + const cursor = encodeCursor(AT, "delivery_1"); + + await app.inject({ + method: "GET", + url: `${url}?cursor=${cursor}`, + headers: authHeader(), + }); + + const args = prisma.webhookDelivery.findMany.mock.calls[0][0]; + expect(args.where.OR).toEqual([ + { createdAt: { lt: AT } }, + { createdAt: AT, id: { lt: "delivery_1" } }, + ]); + }); + + it("rejects a malformed cursor rather than silently restarting", async () => { + const res = await app.inject({ + method: "GET", + url: `${url}?cursor=not-a-real-cursor`, + headers: authHeader(), + }); + + expect(res.statusCode).toBe(400); + expect(prisma.webhookDelivery.findMany).not.toHaveBeenCalled(); + }); + + it("rejects a page size above the maximum instead of clamping it", async () => { + const res = await app.inject({ + method: "GET", + url: `${url}?limit=${MAX_PAGE_SIZE + 1}`, + headers: authHeader(), + }); + + expect(res.statusCode).toBe(400); + expect(prisma.webhookDelivery.findMany).not.toHaveBeenCalled(); + }); + + it("never returns the signed payload or the receiver's response body", async () => { + // Both are on the row. The payload is the group's data re-served through a + // different authorization path, and the response body is third-party text. + prisma.webhookDelivery.findMany.mockResolvedValue([delivery()]); + + const res = await app.inject({ method: "GET", url, headers: authHeader() }); + const [row] = res.json().deliveries; + + expect(row).not.toHaveProperty("payload"); + expect(row).not.toHaveProperty("responseBody"); + expect(res.payload).not.toContain("the exact signed bytes"); + expect(res.payload).not.toContain("something private"); + // The operationally useful fields are still there. + expect(row).toMatchObject({ + id: "delivery_1", + eventType: "expense.created", + status: "failed", + attempts: 3, + responseStatusCode: 500, + }); + }); +}); + +describe("GET /groups/:groupId/webhooks/:webhookId/deliveries — filtering", () => { + const url = `/groups/${GROUP_ID}/webhooks/${WEBHOOK_ID}/deliveries`; + + it("filters by delivery status in the query", async () => { + await app.inject({ method: "GET", url: `${url}?status=failed`, headers: authHeader() }); + + const args = prisma.webhookDelivery.findMany.mock.calls[0][0]; + expect(args.where.AND).toEqual( + expect.arrayContaining([{ webhookId: WEBHOOK_ID }, { status: "failed" }]) + ); + }); + + it("filters by event type in the query", async () => { + await app.inject({ + method: "GET", + url: `${url}?event=expense.created`, + headers: authHeader(), + }); + + const args = prisma.webhookDelivery.findMany.mock.calls[0][0]; + expect(args.where.AND).toEqual( + expect.arrayContaining([{ eventType: "expense.created" }]) + ); + }); + + it("composes a filter with the cursor instead of replacing it", async () => { + const cursor = encodeCursor(AT, "delivery_1"); + + await app.inject({ + method: "GET", + url: `${url}?status=failed&cursor=${cursor}`, + headers: authHeader(), + }); + + const args = prisma.webhookDelivery.findMany.mock.calls[0][0]; + expect(args.where.AND).toHaveLength(3); + expect(args.where.AND[2].OR).toEqual([ + { createdAt: { lt: AT } }, + { createdAt: AT, id: { lt: "delivery_1" } }, + ]); + }); + + it("rejects an unknown status or event rather than returning an empty page", async () => { + for (const query of ["status=exploded", "event=expense.invented"]) { + const res = await app.inject({ + method: "GET", + url: `${url}?${query}`, + headers: authHeader(), + }); + expect(res.statusCode, query).toBe(400); + } + expect(prisma.webhookDelivery.findMany).not.toHaveBeenCalled(); + }); + + it("keeps the endpoint scope out of the cursor's reach", () => { + // The webhook id is always a filter of its own, so a cursor lifted from + // another endpoint's list can only move where a page starts inside data + // the caller may already read. + expect(deliveryFilters(WEBHOOK_ID, {})).toEqual([{ webhookId: WEBHOOK_ID }]); + expect(deliveryFilters(WEBHOOK_ID, { status: "delivered" })).toEqual([ + { webhookId: WEBHOOK_ID }, + { status: "delivered" }, + ]); + }); +}); + +describe("GET /groups/:groupId/webhooks — pagination", () => { + const url = `/groups/${GROUP_ID}/webhooks`; + + it("bounds the read and returns page metadata", async () => { + prisma.webhook.findMany.mockResolvedValue([webhookRow()]); + + const res = await app.inject({ method: "GET", url, headers: authHeader() }); + + expect(res.statusCode).toBe(200); + expect(res.json().meta).toMatchObject({ hasMore: false, nextCursor: null }); + + const args = prisma.webhook.findMany.mock.calls[0][0]; + expect(args.take).toBeGreaterThan(0); + expect(args.orderBy).toEqual([{ createdAt: "desc" }, { id: "desc" }]); + expect(args.where.groupId).toBe(GROUP_ID); + }); + + it("still never returns an endpoint's signing secret", async () => { + prisma.webhook.findMany.mockResolvedValue([webhookRow()]); + + const res = await app.inject({ method: "GET", url, headers: authHeader() }); + + expect(res.payload).not.toContain("never-returned-on-a-list"); + expect(res.json().webhooks[0]).not.toHaveProperty("secret"); + }); + + it("returns an empty page when the group has no endpoints", async () => { + const res = await app.inject({ method: "GET", url, headers: authHeader() }); + + expect(res.statusCode).toBe(200); + expect(res.json().webhooks).toEqual([]); + }); +}); + +describe("GET /groups/:id/settlements — filtering", () => { + const url = `/groups/${GROUP_ID}/settlements`; + + it("keeps the unfiltered query exactly as it was", async () => { + prisma.settlement.findMany.mockResolvedValue([settlementRow()]); + + const res = await app.inject({ method: "GET", url, headers: authHeader() }); + + expect(res.statusCode).toBe(200); + const args = prisma.settlement.findMany.mock.calls[0][0]; + // Flat shape, no AND wrapper: an unfiltered request pays nothing for the + // filters existing. + expect(args.where).toEqual({ groupId: GROUP_ID }); + expect(args.orderBy).toEqual([{ createdAt: "desc" }, { id: "desc" }]); + }); + + it("filters by asset in the query", async () => { + await app.inject({ method: "GET", url: `${url}?asset=USDC`, headers: authHeader() }); + + const args = prisma.settlement.findMany.mock.calls[0][0]; + expect(args.where.AND).toEqual( + expect.arrayContaining([{ groupId: GROUP_ID }, { assetCode: "USDC" }]) + ); + }); + + it("filters by an inclusive date range", async () => { + await app.inject({ + method: "GET", + url: `${url}?startDate=2026-05-01T00:00:00.000Z&endDate=2026-05-31T23:59:59.000Z`, + headers: authHeader(), + }); + + const args = prisma.settlement.findMany.mock.calls[0][0]; + expect(args.where.AND).toEqual( + expect.arrayContaining([ + { + createdAt: { + gte: new Date("2026-05-01T00:00:00.000Z"), + lte: new Date("2026-05-31T23:59:59.000Z"), + }, + }, + ]) + ); + }); + + it("rejects a reversed date range instead of returning nothing", async () => { + const res = await app.inject({ + method: "GET", + url: `${url}?startDate=2026-06-01T00:00:00.000Z&endDate=2026-05-01T00:00:00.000Z`, + headers: authHeader(), + }); + + expect(res.statusCode).toBe(400); + expect(res.json().code).toBe("INVALID_RANGE"); + expect(prisma.settlement.findMany).not.toHaveBeenCalled(); + }); + + it("rejects an unknown status rather than returning an empty page", async () => { + const res = await app.inject({ + method: "GET", + url: `${url}?status=almost_done`, + headers: authHeader(), + }); + + expect(res.statusCode).toBe(400); + expect(prisma.settlement.findMany).not.toHaveBeenCalled(); + }); + + it("counts the filtered set only when the caller asks for a total", async () => { + prisma.settlement.count.mockResolvedValue(42); + + const without = await app.inject({ + method: "GET", + url: `${url}?asset=USDC`, + headers: authHeader(), + }); + expect(without.json().meta.total).toBeUndefined(); + expect(prisma.settlement.count).not.toHaveBeenCalled(); + + const withTotal = await app.inject({ + method: "GET", + url: `${url}?asset=USDC&includeTotal=true`, + headers: authHeader(), + }); + expect(withTotal.json().meta.total).toBe(42); + + // Counted over the filters alone: including the cursor would count only + // the remainder of the scan. + const countArgs = prisma.settlement.count.mock.calls[0][0]; + expect(countArgs.where.AND).toEqual([{ groupId: GROUP_ID }, { assetCode: "USDC" }]); + }); + + it("composes filters with the cursor", async () => { + const cursor = encodeCursor(AT, "settlement_1"); + + await app.inject({ + method: "GET", + url: `${url}?asset=USDC&cursor=${cursor}`, + headers: authHeader(), + }); + + const args = prisma.settlement.findMany.mock.calls[0][0]; + expect(args.where.AND[args.where.AND.length - 1].OR).toEqual([ + { createdAt: { lt: AT } }, + { createdAt: AT, id: { lt: "settlement_1" } }, + ]); + }); +}); + +/** + * The public status filter is the inverse of `toPublicStatus`, so it gets its + * own cases: a filter that disagreed with the status a settlement reports when + * read individually would be worse than no filter at all. + */ +describe("settlement status filtering is the inverse of the public status", () => { + const now = new Date("2026-05-01T12:00:00.000Z"); + const cutoff = expiryCutoff(now); + + it("derives the cutoff with the same clock-skew tolerance as the expiry check", () => { + expect(cutoff.getTime()).toBe(now.getTime() - CLOCK_SKEW_TOLERANCE_SECONDS * 1000); + }); + + it("matches a stored terminal status directly, whatever the clock reads", () => { + expect(publicStatusFilter("confirmed", now)).toEqual({ status: "confirmed" }); + expect(publicStatusFilter("failed", now)).toEqual({ status: "failed" }); + }); + + it("treats a lapsed in-flight row as expired, not as still in flight", () => { + const filter = publicStatusFilter("expired", now) as any; + expect(filter.OR[0]).toEqual({ status: "expired" }); + expect(filter.OR[1]).toEqual({ + status: { notIn: ["confirmed", "failed", "expired"] }, + expiresAt: { lt: cutoff }, + }); + }); + + it("requires a live window for submitted and awaiting_signature", () => { + const live = { OR: [{ expiresAt: null }, { expiresAt: { gte: cutoff } }] }; + expect(publicStatusFilter("submitted", now)).toEqual({ + AND: [{ status: "submitted" }, live], + }); + expect(publicStatusFilter("awaiting_signature", now)).toEqual({ + AND: [ + { status: { notIn: ["confirmed", "failed", "expired", "submitted"] } }, + live, + ], + }); + }); + + it("puts an unrecognised column value in awaiting_signature, as the mapping does", () => { + // `toPublicStatus` reports anything it does not know as awaiting_signature, + // so the filter is written with `notIn` rather than `equals: "pending"` — + // otherwise a row with a future status value would be unreachable through + // every one of the five filters. + const filter = publicStatusFilter("awaiting_signature", now) as any; + expect(filter.AND[0].status.notIn).not.toContain("pending"); + }); + + it("scopes by group independently of any status filter", () => { + expect(settlementFilters(GROUP_ID, {}, now)).toEqual([{ groupId: GROUP_ID }]); + expect(settlementFilters(GROUP_ID, { status: "confirmed" }, now)[0]).toEqual({ + groupId: GROUP_ID, + }); + }); +}); + +describe("GET /groups/:id/ledger — filtering", () => { + const url = `/groups/${GROUP_ID}/ledger`; + + it("reads all three tables when no type is named", async () => { + await app.inject({ method: "GET", url, headers: authHeader() }); + + expect(prisma.expense.findMany).toHaveBeenCalled(); + expect(prisma.settlement.findMany).toHaveBeenCalled(); + expect(prisma.treasuryTransaction.findMany).toHaveBeenCalled(); + }); + + it("reads only the requested table for a single type", async () => { + await app.inject({ method: "GET", url: `${url}?type=expense`, headers: authHeader() }); + + expect(prisma.expense.findMany).toHaveBeenCalled(); + expect(prisma.settlement.findMany).not.toHaveBeenCalled(); + expect(prisma.treasuryTransaction.findMany).not.toHaveBeenCalled(); + }); + + it("accepts a comma-separated list of types", async () => { + await app.inject({ + method: "GET", + url: `${url}?type=expense,treasury`, + headers: authHeader(), + }); + + expect(prisma.expense.findMany).toHaveBeenCalled(); + expect(prisma.treasuryTransaction.findMany).toHaveBeenCalled(); + expect(prisma.settlement.findMany).not.toHaveBeenCalled(); + }); + + it("returns only entries of the requested type", async () => { + prisma.expense.findMany.mockResolvedValue([expenseRow()]); + + const body = ( + await app.inject({ method: "GET", url: `${url}?type=expense`, headers: authHeader() }) + ).json(); + + expect(body.entries).toHaveLength(1); + expect(body.entries[0].type).toBe("expense"); + expect(body.meta).toMatchObject({ hasMore: false, nextCursor: null }); + }); + + it("pushes the date bound into every table it reads", async () => { + await app.inject({ + method: "GET", + url: `${url}?startDate=2026-05-01T00:00:00.000Z`, + headers: authHeader(), + }); + + for (const model of [ + prisma.expense.findMany, + prisma.settlement.findMany, + prisma.treasuryTransaction.findMany, + ]) { + const args = model.mock.calls[0][0]; + expect(args.where.createdAt).toEqual({ + gte: new Date("2026-05-01T00:00:00.000Z"), + }); + // The bound is a filter alongside the cursor, so each table is still + // read with its own bounded window. + expect(args.take).toBeGreaterThan(0); + } + }); + + it("keeps the unfiltered query exactly as it was", async () => { + await app.inject({ method: "GET", url, headers: authHeader() }); + + const args = prisma.expense.findMany.mock.calls[0][0]; + expect(args.where).toEqual({ groupId: GROUP_ID }); + }); + + it("rejects an unknown type rather than returning an empty ledger", async () => { + const res = await app.inject({ + method: "GET", + url: `${url}?type=invoice`, + headers: authHeader(), + }); + + expect(res.statusCode).toBe(400); + expect(prisma.expense.findMany).not.toHaveBeenCalled(); + }); + + it("rejects a reversed date range", async () => { + const res = await app.inject({ + method: "GET", + url: `${url}?startDate=2026-06-01T00:00:00.000Z&endDate=2026-05-01T00:00:00.000Z`, + headers: authHeader(), + }); + + expect(res.statusCode).toBe(400); + expect(prisma.expense.findMany).not.toHaveBeenCalled(); + }); + + it("de-duplicates a repeated type so no table is read twice", () => { + expect(ledgerQuerySchema.parse({ type: "expense,expense" }).type).toEqual(["expense"]); + expect(ledgerQuerySchema.parse({ type: " expense , settlement " }).type).toEqual([ + "expense", + "settlement", + ]); + expect(ledgerQuerySchema.parse({}).type).toBeUndefined(); + }); +}); diff --git a/tests/services/xdr-inspection.test.ts b/tests/services/xdr-inspection.test.ts new file mode 100644 index 00000000..dba6c5c9 --- /dev/null +++ b/tests/services/xdr-inspection.test.ts @@ -0,0 +1,417 @@ +/** + * Issue #694 — signed-XDR inspection against a stored settlement intent. + * + * Mergepay never holds a private key: it builds an unsigned payment, a wallet + * signs it, and a later request hands the envelope back. Every field a wallet + * could change in that gap is checked here, with sample envelopes built by the + * same `stellar.buildPayment` the API serves to clients, so a fixture cannot + * drift from what a real wallet is asked to sign. + * + * The suite asserts three things the module exists for: + * + * 1. the accepted case returns the parsed envelope, its hash, and the source + * and sequence it declares; + * 2. every divergence is reported with its own `field`, not just prose, and + * the rejection never echoes the envelope's contents back; + * 3. an unsigned or wrongly signed envelope is rejected — the check the + * removed `src/utils/stellar-xdr.ts` validator never performed, which is + * how a "validated" envelope could carry nobody's authorization. + * + * Shape-level checks (source, sequence, operation count, fee band) are + * asserted against the shared matcher's order: an envelope with two operations + * must be reported as such rather than as an incidental asset or amount + * mismatch further down. + */ +import { describe, it, expect } from "vitest"; +import { + Account, + Asset, + BASE_FEE, + Keypair, + Memo, + Operation, + Transaction, + TransactionBuilder, +} from "@stellar/stellar-sdk"; +import { + assertSignedTransactionMatchesIntent, + inspectSignedTransaction, +} from "../../src/services/xdr-inspection"; +import { stellar, type PaymentExpectation } from "../../src/services/stellar"; +import { AppError } from "../../src/errors/app-error"; +import { config } from "../../src/config"; + +const payer = Keypair.random(); +const recipient = Keypair.random(); +/** The account sequence Horizon reported when the intent was built. */ +const ACCOUNT_SEQUENCE = "12345"; +/** The envelope's own sequence: one past the account's, as the builder sets it. */ +const TX_SEQUENCE = "12346"; + +/** The stored intent every envelope below is judged against. */ +const intent: PaymentExpectation = { + sourcePublicKey: payer.publicKey(), + destination: recipient.publicKey(), + asset: { code: "XLM", issuer: null }, + amount: "50.0000000", + memoCode: "ABC234", + resource: "settlement", +}; + +/** An unsigned envelope for the intent, optionally with fields diverging. */ +function buildXdr(overrides: Partial = {}): string { + return stellar.buildPayment({ + sourcePublicKey: overrides.sourcePublicKey ?? intent.sourcePublicKey, + // `buildPayment` takes the *account* sequence; the envelope it returns + // carries TX_SEQUENCE. + sourceSequence: ACCOUNT_SEQUENCE, + destination: overrides.destination ?? intent.destination, + asset: overrides.asset ?? intent.asset, + amount: overrides.amount ?? intent.amount, + memoCode: overrides.memoCode ?? intent.memoCode, + }); +} + +/** Sign an envelope with `signer` (the payer unless told otherwise). */ +function sign(xdr: string, signer: Keypair = payer): string { + const tx = new Transaction(xdr, config.networkPassphrase); + tx.sign(signer); + return tx.toXDR(); +} + +/** A hand-built envelope, for shapes `buildPayment` will not produce. */ +function buildCustomXdr( + options: { + fee?: string; + operations?: number; + operationSource?: string; + memo?: string | null; + operationType?: "payment" | "manageData"; + asset?: Asset; + } = {} +): string { + const { + fee = BASE_FEE, + operations = 1, + operationSource, + memo = `MP:${intent.memoCode}`, + operationType = "payment", + asset = Asset.native(), + } = options; + + const builder = new TransactionBuilder( + new Account(intent.sourcePublicKey, ACCOUNT_SEQUENCE), + { fee, networkPassphrase: config.networkPassphrase } + ); + + for (let i = 0; i < operations; i++) { + builder.addOperation( + operationType === "manageData" + ? Operation.manageData({ name: "not-a-payment", value: "x" }) + : Operation.payment({ + source: operationSource, + destination: intent.destination, + asset, + amount: intent.amount, + }) + ); + } + + if (memo !== null) builder.addMemo(Memo.text(memo)); + return builder.setTimeout(300).build().toXDR(); +} + +describe("inspectSignedTransaction — accepted envelope", () => { + it("accepts the signed envelope the API itself built", () => { + const signedXdr = sign(buildXdr()); + const result = inspectSignedTransaction(signedXdr, intent); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.source).toBe(payer.publicKey()); + expect(result.sequence).toBe(TX_SEQUENCE); + expect(result.hash).toBe( + new Transaction(signedXdr, config.networkPassphrase).hash().toString("hex") + ); + }); + + it("accepts an amount written with different trailing precision", () => { + // "50" and "50.0000000" are the same payment; a raw string comparison + // would reject one of them, which is a real wallet behaviour. + const result = inspectSignedTransaction(sign(buildXdr({ amount: "50" })), intent); + expect(result).toMatchObject({ ok: true }); + }); + + it("accepts an unsigned envelope only when the caller opts out of the check", () => { + const unsigned = buildXdr(); + expect(inspectSignedTransaction(unsigned, intent)).toMatchObject({ + ok: false, + code: "xdr_unsigned", + }); + expect( + inspectSignedTransaction(unsigned, intent, { requireSignature: false }) + ).toMatchObject({ ok: true }); + }); + + it("checks the sequence only when the intent recorded one", () => { + const signedXdr = sign(buildXdr()); + expect( + inspectSignedTransaction(signedXdr, { ...intent, sourceSequence: TX_SEQUENCE }) + ).toMatchObject({ ok: true }); + expect( + inspectSignedTransaction(signedXdr, { ...intent, sourceSequence: "999" }) + ).toMatchObject({ ok: false, field: "sequence" }); + }); +}); + +describe("inspectSignedTransaction — signature verification", () => { + it("rejects an unsigned envelope", () => { + expect(inspectSignedTransaction(buildXdr(), intent)).toMatchObject({ + ok: false, + code: "xdr_unsigned", + field: "signature", + }); + }); + + it("rejects an envelope signed by someone other than the source account", () => { + const result = inspectSignedTransaction(sign(buildXdr(), Keypair.random()), intent); + expect(result).toMatchObject({ ok: false, code: "xdr_unsigned", field: "signature" }); + }); + + it("rejects a signature produced for a different network", () => { + // The transaction hash is computed over bytes that include the network + // passphrase, so a testnet signature cannot verify against a public-network + // envelope even with the right key. + const unsigned = buildXdr(); + const otherNetwork = new Transaction(unsigned, "Other Network ; 2037"); + otherNetwork.sign(payer); + const signatures = otherNetwork.signatures; + + const tx = new Transaction(unsigned, config.networkPassphrase); + for (const signature of signatures) tx.signatures.push(signature); + + expect(inspectSignedTransaction(tx.toXDR(), intent)).toMatchObject({ + ok: false, + code: "xdr_unsigned", + }); + }); +}); + +describe("inspectSignedTransaction — intent divergence", () => { + it("reports a different source account", () => { + const other = Keypair.random(); + const xdr = buildXdr({ sourcePublicKey: other.publicKey() }); + expect(inspectSignedTransaction(sign(xdr, other), intent)).toMatchObject({ + ok: false, + code: "xdr_mismatch", + field: "source", + }); + }); + + it("reports a different destination", () => { + const xdr = buildXdr({ destination: Keypair.random().publicKey() }); + expect(inspectSignedTransaction(sign(xdr), intent)).toMatchObject({ + ok: false, + code: "xdr_mismatch", + field: "destination", + }); + }); + + it("reports a different amount", () => { + expect( + inspectSignedTransaction(sign(buildXdr({ amount: "999.0000000" })), intent) + ).toMatchObject({ ok: false, code: "xdr_mismatch", field: "amount" }); + }); + + it("reports a different asset", () => { + const usdc = { code: config.STABLE_ASSET_CODE, issuer: config.STABLE_ASSET_ISSUER }; + expect( + inspectSignedTransaction(sign(buildXdr({ asset: usdc })), intent) + ).toMatchObject({ ok: false, code: "xdr_mismatch", field: "asset" }); + }); + + it("reports an issuer swap separately from a code mismatch", () => { + // Same familiar asset code, a different issuer: the recipient receives a + // different asset entirely, and nothing in the amount or the memo shows + // it — so it gets its own field rather than the generic asset mismatch. + const impostor = new Asset(config.STABLE_ASSET_CODE, Keypair.random().publicKey()); + const xdr = buildCustomXdr({ asset: impostor }); + const result = inspectSignedTransaction(sign(xdr), { + ...intent, + asset: { code: config.STABLE_ASSET_CODE, issuer: config.STABLE_ASSET_ISSUER }, + }); + expect(result).toMatchObject({ + ok: false, + code: "xdr_mismatch", + field: "assetIssuer", + }); + }); + + it("reports a memo that does not name the stored reference", () => { + expect( + inspectSignedTransaction(sign(buildXdr({ memoCode: "ZZZ999" })), intent) + ).toMatchObject({ ok: false, code: "xdr_mismatch", field: "memo" }); + }); + + it("reports a missing memo", () => { + expect( + inspectSignedTransaction(sign(buildCustomXdr({ memo: null })), intent) + ).toMatchObject({ ok: false, code: "xdr_mismatch", field: "memo" }); + }); +}); + +describe("inspectSignedTransaction — unexpected operations", () => { + it("rejects an envelope carrying more than one operation", () => { + expect( + inspectSignedTransaction(sign(buildCustomXdr({ operations: 2, fee: "200" })), intent) + ).toMatchObject({ ok: false, code: "xdr_mismatch", field: "operationCount" }); + }); + + it("rejects an operation that is not a payment", () => { + expect( + inspectSignedTransaction( + sign(buildCustomXdr({ operationType: "manageData" })), + intent + ) + ).toMatchObject({ ok: false, code: "xdr_mismatch", field: "operationType" }); + }); + + it("rejects a payment whose operation source is a third account", () => { + // An operation-level source lets an envelope spend a *different* account + // than the transaction declares, which is not the payment we authorized. + const xdr = buildCustomXdr({ operationSource: Keypair.random().publicKey() }); + expect(inspectSignedTransaction(sign(xdr), intent)).toMatchObject({ + ok: false, + code: "xdr_mismatch", + field: "operationSource", + }); + }); + + it("rejects a fee outside the accepted band", () => { + expect( + inspectSignedTransaction(sign(buildCustomXdr({ fee: "100000" })), intent) + ).toMatchObject({ ok: false, code: "xdr_mismatch", field: "fee" }); + }); +}); + +describe("inspectSignedTransaction — malformed and expired envelopes", () => { + it.each(["", " ", "not-xdr!!", "signed XDR goes here"])( + "rejects input that is not an envelope at all: %j", + (value) => { + expect(inspectSignedTransaction(value, intent)).toMatchObject({ + ok: false, + code: "xdr_malformed", + field: "envelope", + }); + } + ); + + it("rejects a fee-bump wrapper as a different transaction, not a bad operation count", () => { + const inner = new Transaction(buildXdr(), config.networkPassphrase); + inner.sign(payer); + const feeBump = TransactionBuilder.buildFeeBumpTransaction( + Keypair.random().publicKey(), + String(Number(BASE_FEE) * 2), + inner, + config.networkPassphrase + ); + + const result = inspectSignedTransaction(feeBump.toXDR(), intent); + expect(result).toMatchObject({ ok: false, code: "xdr_mismatch", field: "envelope" }); + if (result.ok) return; + expect(result.message).toMatch(/fee-bump/i); + }); + + it("reports an envelope whose validity window has lapsed as expired", () => { + const expired = new TransactionBuilder( + new Account(intent.sourcePublicKey, ACCOUNT_SEQUENCE), + { + fee: BASE_FEE, + networkPassphrase: config.networkPassphrase, + timebounds: { minTime: 0, maxTime: Math.floor(Date.now() / 1000) - 3600 }, + } + ) + .addOperation( + Operation.payment({ + destination: intent.destination, + asset: Asset.native(), + amount: intent.amount, + }) + ) + .addMemo(Memo.text(`MP:${intent.memoCode}`)) + .build(); + expired.sign(payer); + + expect(inspectSignedTransaction(expired.toXDR(), intent)).toMatchObject({ + ok: false, + code: "intent_expired", + field: "timeBounds", + }); + }); + + it("rejects an envelope that never expires", () => { + const unbounded = new TransactionBuilder( + new Account(intent.sourcePublicKey, ACCOUNT_SEQUENCE), + { fee: BASE_FEE, networkPassphrase: config.networkPassphrase } + ) + .addOperation( + Operation.payment({ + destination: intent.destination, + asset: Asset.native(), + amount: intent.amount, + }) + ) + .addMemo(Memo.text(`MP:${intent.memoCode}`)) + .setTimeout(0) + .build(); + unbounded.sign(payer); + + expect(inspectSignedTransaction(unbounded.toXDR(), intent)).toMatchObject({ + ok: false, + field: "timeBounds", + }); + }); +}); + +describe("inspectSignedTransaction — what a rejection may disclose", () => { + it("never echoes the envelope or its values back in the message", () => { + const signedXdr = sign(buildXdr({ amount: "999.0000000" })); + const result = inspectSignedTransaction(signedXdr, intent); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.message).not.toContain("999"); + expect(result.message).not.toContain(signedXdr.slice(0, 24)); + expect(result.message).not.toContain(payer.publicKey()); + }); +}); + +describe("assertSignedTransactionMatchesIntent", () => { + it("returns the accepted inspection for a valid envelope", () => { + const result = assertSignedTransactionMatchesIntent(sign(buildXdr()), intent); + expect(result.source).toBe(payer.publicKey()); + expect(result.hash).toHaveLength(64); + }); + + it("throws a 400 carrying the stable code and the diverging field", () => { + try { + assertSignedTransactionMatchesIntent(sign(buildXdr({ amount: "1" })), intent); + throw new Error("expected a rejection"); + } catch (error) { + expect(error).toBeInstanceOf(AppError); + const appError = error as AppError; + expect(appError.status).toBe(400); + expect(appError.code).toBe("XDR_MISMATCH"); + expect(appError.details).toEqual({ field: "amount" }); + } + }); + + it("throws XDR_UNSIGNED for an envelope nobody signed", () => { + try { + assertSignedTransactionMatchesIntent(buildXdr(), intent); + throw new Error("expected a rejection"); + } catch (error) { + expect((error as AppError).code).toBe("XDR_UNSIGNED"); + } + }); +});