diff --git a/.env.example b/.env.example index 145aa6d..d3de7b2 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 0cb50dd..a44c8f9 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 5482bab..17f443e 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 46f8b40..4590853 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 8622c96..007b89c 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 dad11c4..51e6914 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 1685a66..a912234 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 d8c5293..5738ebd 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 f9847e7..31ad605 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 0000000..0108821 --- /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 0000000..a4d5d68 --- /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 8e00945..43eb9b6 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 0000000..d555b46 --- /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 0000000..b557ddf --- /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 1f871af..e44298d 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 467d9ab..d9a1782 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 7259f3d..96ca418 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 ec2c810..f9bed00 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 0000000..5b86ba1 --- /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 0000000..dba6c5c --- /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"); + } + }); +});