diff --git a/docs/APPROVALS.md b/docs/APPROVALS.md new file mode 100644 index 0000000..c72fa2b --- /dev/null +++ b/docs/APPROVALS.md @@ -0,0 +1,150 @@ +# Approval Workflows & Multi-Signature Governance + +Closes the gap between "requested" and "executed" for high-value or sensitive +operations (#314). Today a user — or a parent acting through a sub-account — +can move funds with zero friction and zero oversight. An `ApprovalPolicy` lets +a principal require `minApprovers` independent co-signers before an operation +above `highValueThreshold` (or every operation, if the threshold is null) is +allowed to submit on-chain. + +## Data model + +| Model | Meaning | +| ----------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `ApprovalPolicy` | Governs one `SubAccountPermission` (`WITHDRAW`/`DEPOSIT`/`MANAGE_STRATEGY`) for a principal's own account (`scopedToChildUserId = null`) or a specific sub-account relationship (`scopedToChildUserId` = the child). | +| `ApprovalRequest` | One held operation. Snapshots `minApprovers` from the policy at request time and carries the exact operation to (re-)run as `payload`. Status machine below. | +| `Approval` | One approver's decision. `@@unique([requestId, approverUserId])` is the concurrency primitive — see below. | + +Status machine: `PENDING → APPROVED → EXECUTED` (terminal — on-chain finality, +never un-executed), or `PENDING → REJECTED / EXPIRED / CANCELLED`. + +## Policy resolution + +| Caller | Resolves | +| ---------------------------------------------------------------------- | ------------------------------------------------------------------------------------------- | +| Self (`actingAsUserId === userId`) | `principalUserId = userId`, `scopedToChildUserId = null` | +| Parent acting on a child (delegated via `requireSubAccountPermission`) | `principalUserId = actingAsUserId` (the parent), `scopedToChildUserId = userId` (the child) | + +Pending requests follow the **policy snapshot at request time** +(`minApprovers` is copied onto the row). Editing or deactivating a policy +never changes an already-open request — only new requests see the change. + +## Eligible approvers + +The issue's schema has no separate "approver list" field. This +implementation reuses the existing `SubAccount` graph as the approver pool: +**the policy owner (`principalUserId`) plus every ACTIVE child under that +same parent** — so "the parent and a child, or two children" can co-sign an +operation on a shared vault, per the issue's example, with no schema change. + +**Self-approval**: the requester (`ApprovalRequest.actingAsUserId`) never +counts toward the threshold when `minApprovers > 1` — hard rule, no +per-policy opt-in in v1 (issue's documented default). + +**Approver eligibility is checked live**, at decision time, against the +current `SubAccount` state — not the policy snapshot. If a family member's +access was revoked between the request being opened and them approving it, +they can no longer approve. An approval already recorded before revocation +still counts toward the threshold (the issue's recommended "count it, the +policy snapshot is the request's" option) — decisions are never retroactively +invalidated once persisted. + +## Enforcement in the money path + +`guardOperation` is called from **inside** `executeDeposit` and +`executeWithdraw` (`src/controllers/transaction-controller.ts`), not from +routes — so every caller that reuses those functions is covered by +construction: + +- `POST /api/v1/withdraw`, `POST /api/v1/deposit` (HTTP) +- `src/jobs/recurringDeposits.ts` (calls `executeDeposit` directly) + +When gated, no `Transaction` row is created yet — the `ApprovalRequest.payload` +holds the exact operation, re-run through the same `executeDeposit`/ +`executeWithdraw` path on approval (`skipApprovalGuard: true`, set only by +`src/approvals/executors.ts`, so the approved re-run can't re-gate itself). +The resulting `Transaction`'s memo is tagged `(approval:)` and +`ApprovalRequest.executedTxId` links back to it, for audit and the tax report. + +**Recurring deposits** treat `PENDING_APPROVAL` as _skip, not fail_: +`lastRunStatus` is set to `pending_approval`, `nextRunAt` is left untouched, +and `guardOperation`'s dedupe check (an existing PENDING request for the same +policy/user/amount) means the next sweep lands on the same open request +instead of creating a new one every tick. + +### Scoped out of v1 (documented, not silently dropped) + +- **The agent rebalance loop** (`src/agent/loop.ts` → + `executeRebalanceIfNeeded`) is not gated. Rebalances move funds + protocol-to-protocol _inside_ the vault (same asset, same user, no + egress) — the same category the tax report already excludes from + disposals — not a withdraw/deposit call. `ApprovalPolicy.permission` still + accepts `MANAGE_STRATEGY` so a policy row is valid data for a future + rebalance-gating change; wiring it into the hourly autonomous loop is a + materially different design problem (a diff-of-allocations, not a single + amount + payload). +- **Referral payouts** (`src/jobs/referralPayout.ts`) are not gated — they + are platform-funded credits _to_ the user, not the user moving funds out, + so they don't fit the delegated-authority threat model this issue is + scoped to (a compromised session, or an over-permissioned family member, + draining a vault). + +## Concurrency + +Every status transition after `PENDING` is a **conditional `updateMany`** +(`where: { id, status: 'PENDING' }`), never read-then-write: + +- Two approvals landing simultaneously: `Approval` creation is guarded by the + `@@unique([requestId, approverUserId])` constraint (a second decision from + the same approver gets `409`, not silently ignored or double-counted). +- Threshold crossed by two concurrent `decide()` calls: both recompute the + approval count and both attempt `updateMany(PENDING → APPROVED)`; only one + `count === 1` — that call proceeds to execute, the other sees `count === 0` + and returns without executing. **Never double-executes.** +- The expiry sweep batches a single `updateMany` per tick since it has no + other actor contending for the same PENDING → EXPIRED transition (unlike + `decide`, which races a concurrent human decision). + +If execution throws or the on-chain call fails after the request reaches +`APPROVED`, the request is deliberately **left `APPROVED`**, not silently +reset to `PENDING` or lost — an ops-visible stuck state for retry/ +investigation, matching the tax module's "never invert the dependency" +philosophy for money-adjacent bookkeeping. + +## Endpoints + +``` +GET /api/v1/approvals — requests affecting the caller (paginated) +GET /api/v1/approvals/:id — full request + decisions +POST /api/v1/approvals/:id/approve — { note? } +POST /api/v1/approvals/:id/reject — { reason } (required) +POST /api/v1/approvals/:id/cancel — requester only +POST /api/v1/admin/approvals/:id/cancel — admin (scope: approvals:write) + +GET /api/v1/approval-policies — policies the caller owns +GET /api/v1/approval-policies/:id +POST /api/v1/approval-policies — { scopedToChildUserId?, permission, minApprovers, highValueThreshold?, approvalTimeoutMs } +PUT /api/v1/approval-policies/:id — { minApprovers?, highValueThreshold?, approvalTimeoutMs?, isActive? } +``` + +Creating/editing a policy with `scopedToChildUserId` set requires the caller +to currently hold `MANAGE_STRATEGY` on that child (re-checked on every write, +not just at creation). + +## Webhooks + +`approval.requested`, `approval.approved`, `approval.rejected`, +`approval.executed`, `approval.expired`, `approval.cancelled` — see +`src/validators/webhook-validators.ts`. + +## Known limitations (v1) + +1. Agent-loop rebalances and referral payouts are not gated (see above). +2. No per-policy self-approval opt-in — always disallowed when + `minApprovers > 1`. +3. Approver pool is derived entirely from the `SubAccount` graph; there is no + standalone "add an approver who isn't a sub-account party" concept. +4. `guardOperation`'s dedupe window is keyed on (policy, user, actingAsUser, + permission, asset, amount) — two _different_ legitimate operations that + happen to share every one of those fields within the same open window + will be treated as the same request. diff --git a/prisma/migrations/20260824153458_add_approval_workflows/migration.sql b/prisma/migrations/20260824153458_add_approval_workflows/migration.sql new file mode 100644 index 0000000..c150e97 --- /dev/null +++ b/prisma/migrations/20260824153458_add_approval_workflows/migration.sql @@ -0,0 +1,82 @@ +-- CreateEnum +CREATE TYPE "ApprovalStatus" AS ENUM ('PENDING', 'APPROVED', 'EXECUTED', 'REJECTED', 'EXPIRED', 'CANCELLED'); + +-- CreateTable +CREATE TABLE "approval_policies" ( + "id" TEXT NOT NULL, + "principalUserId" TEXT NOT NULL, + "scopedToChildUserId" TEXT, + "permission" "SubAccountPermission" NOT NULL, + "minApprovers" INTEGER NOT NULL, + "highValueThreshold" DECIMAL(36,18), + "approvalTimeoutMs" INTEGER NOT NULL, + "isActive" BOOLEAN NOT NULL DEFAULT true, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updatedAt" TIMESTAMP(3) NOT NULL, + + CONSTRAINT "approval_policies_pkey" PRIMARY KEY ("id") +); + +-- CreateTable +CREATE TABLE "approval_requests" ( + "id" TEXT NOT NULL, + "policyId" TEXT NOT NULL, + "userId" TEXT NOT NULL, + "actingAsUserId" TEXT NOT NULL, + "permission" "SubAccountPermission" NOT NULL, + "amount" DECIMAL(36,18) NOT NULL, + "assetSymbol" TEXT NOT NULL, + "payload" JSONB NOT NULL, + "status" "ApprovalStatus" NOT NULL DEFAULT 'PENDING', + "minApprovers" INTEGER NOT NULL, + "requestedAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "expiresAt" TIMESTAMP(3) NOT NULL, + "executedAt" TIMESTAMP(3), + "executedTxId" TEXT, + "cancelledById" TEXT, + "reason" TEXT, + + CONSTRAINT "approval_requests_pkey" PRIMARY KEY ("id") +); + +-- CreateTable +CREATE TABLE "approvals" ( + "id" TEXT NOT NULL, + "requestId" TEXT NOT NULL, + "approverUserId" TEXT NOT NULL, + "approved" BOOLEAN NOT NULL, + "note" TEXT, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "approvals_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE INDEX "approval_policies_principalUserId_permission_isActive_idx" ON "approval_policies"("principalUserId", "permission", "isActive"); + +-- CreateIndex +CREATE INDEX "approval_policies_scopedToChildUserId_permission_isActive_idx" ON "approval_policies"("scopedToChildUserId", "permission", "isActive"); + +-- CreateIndex +CREATE INDEX "approval_requests_userId_status_idx" ON "approval_requests"("userId", "status"); + +-- CreateIndex +CREATE INDEX "approval_requests_status_expiresAt_idx" ON "approval_requests"("status", "expiresAt"); + +-- CreateIndex +CREATE INDEX "approvals_requestId_idx" ON "approvals"("requestId"); + +-- CreateIndex +CREATE UNIQUE INDEX "approvals_requestId_approverUserId_key" ON "approvals"("requestId", "approverUserId"); + +-- AddForeignKey +ALTER TABLE "approval_policies" ADD CONSTRAINT "approval_policies_principalUserId_fkey" FOREIGN KEY ("principalUserId") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "approval_requests" ADD CONSTRAINT "approval_requests_policyId_fkey" FOREIGN KEY ("policyId") REFERENCES "approval_policies"("id") ON DELETE RESTRICT ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "approval_requests" ADD CONSTRAINT "approval_requests_userId_fkey" FOREIGN KEY ("userId") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "approvals" ADD CONSTRAINT "approvals_requestId_fkey" FOREIGN KEY ("requestId") REFERENCES "approval_requests"("id") ON DELETE CASCADE ON UPDATE CASCADE; diff --git a/prisma/migrations/20260824153458_add_approval_workflows/rollback.sql b/prisma/migrations/20260824153458_add_approval_workflows/rollback.sql new file mode 100644 index 0000000..3da339f --- /dev/null +++ b/prisma/migrations/20260824153458_add_approval_workflows/rollback.sql @@ -0,0 +1,18 @@ +-- Rollback for 20260824153458_add_approval_workflows +-- Drops the approval-workflow tables (#314). +-- WARNING: Destroys every ApprovalPolicy/ApprovalRequest/Approval row — +-- any currently-open PENDING_APPROVAL operation's intent is lost, not just +-- deferred. Deploy the reverted application code BEFORE running this: the +-- live code calls guardOperation on every deposit/withdraw, so dropping +-- these tables underneath a running server breaks that gate. + +ALTER TABLE "approvals" DROP CONSTRAINT IF EXISTS "approvals_requestId_fkey"; +ALTER TABLE "approval_requests" DROP CONSTRAINT IF EXISTS "approval_requests_userId_fkey"; +ALTER TABLE "approval_requests" DROP CONSTRAINT IF EXISTS "approval_requests_policyId_fkey"; +ALTER TABLE "approval_policies" DROP CONSTRAINT IF EXISTS "approval_policies_principalUserId_fkey"; + +DROP TABLE IF EXISTS "approvals"; +DROP TABLE IF EXISTS "approval_requests"; +DROP TABLE IF EXISTS "approval_policies"; + +DROP TYPE IF EXISTS "ApprovalStatus"; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index e46346c..e6c88f5 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -225,6 +225,8 @@ model User { allocationSuggestions AllocationSuggestion[] portfolioAttributions PortfolioAttribution[] portfolioRiskAggregates PortfolioRiskAggregate[] + approvalPolicies ApprovalPolicy[] @relation("ApprovalPolicyPrincipal") + approvalRequests ApprovalRequest[] @relation("ApprovalRequestPrincipal") userEvents UserEvent[] userEventSequence UserEventSequence? userApiKeys UserApiKey[] @@ -823,6 +825,96 @@ model SubAccount { @@map("sub_accounts") } +enum ApprovalStatus { + PENDING + APPROVED + EXECUTED + REJECTED + EXPIRED + CANCELLED +} + +/// Multi-signature approval policy (#314). Governs one permission class +/// (WITHDRAW/DEPOSIT/MANAGE_STRATEGY) for a principal's own account +/// (scopedToChildUserId = null) or for a specific sub-account relationship +/// (scopedToChildUserId = the child). A null highValueThreshold means the +/// policy applies to every operation of that permission, not just large ones. +/// Pending requests snapshot the policy that created them (see +/// ApprovalRequest.minApprovers) — editing or deactivating a policy never +/// changes an already-open request, only new ones. +model ApprovalPolicy { + id String @id @default(uuid()) + principalUserId String + scopedToChildUserId String? + permission SubAccountPermission + minApprovers Int + highValueThreshold Decimal? @db.Decimal(36, 18) + approvalTimeoutMs Int + isActive Boolean @default(true) + createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt + + principal User @relation("ApprovalPolicyPrincipal", fields: [principalUserId], references: [id], onDelete: Cascade) + requests ApprovalRequest[] + + @@index([principalUserId, permission, isActive]) + @@index([scopedToChildUserId, permission, isActive]) + @@map("approval_policies") +} + +/// One high-value/sensitive operation held at PENDING_APPROVAL. Status +/// machine: PENDING -> APPROVED -> EXECUTED (terminal, on-chain finality, +/// never un-executed); PENDING -> REJECTED / EXPIRED / CANCELLED. `payload` +/// is the exact operation to re-run on execution (see +/// src/approvals/service.ts's guardOperation/decide). `minApprovers` is +/// snapshotted from the policy at request time so a later policy edit never +/// changes an open request's threshold. +model ApprovalRequest { + id String @id @default(uuid()) + policyId String + userId String + actingAsUserId String + permission SubAccountPermission + amount Decimal @db.Decimal(36, 18) + assetSymbol String + payload Json + status ApprovalStatus @default(PENDING) + minApprovers Int + requestedAt DateTime @default(now()) + expiresAt DateTime + executedAt DateTime? + executedTxId String? + cancelledById String? + reason String? + + policy ApprovalPolicy @relation(fields: [policyId], references: [id]) + user User @relation("ApprovalRequestPrincipal", fields: [userId], references: [id], onDelete: Cascade) + approvals Approval[] + + @@index([userId, status]) + @@index([status, expiresAt]) + @@map("approval_requests") +} + +/// One approver's decision on an ApprovalRequest. The unique constraint is +/// the concurrency primitive: two approvals landing simultaneously cannot +/// both be recorded for the same (request, approver) pair, and threshold-met +/// detection is a conditional UPDATE on the request, never check-then-act. +model Approval { + id String @id @default(uuid()) + requestId String + approverUserId String + approved Boolean + note String? + createdAt DateTime @default(now()) + + request ApprovalRequest @relation(fields: [requestId], references: [id], onDelete: Cascade) + + @@unique([requestId, approverUserId]) + @@index([requestId]) + @@map("approvals") +} + /// Fiat on-ramp / off-ramp order (#290). /// Tracks the off-chain payment leg of a fiat<->crypto conversion handled by a /// third-party provider (e.g. MoonPay). The on-chain settlement is reconciled diff --git a/src/approvals/executors.ts b/src/approvals/executors.ts new file mode 100644 index 0000000..1d0a8f6 --- /dev/null +++ b/src/approvals/executors.ts @@ -0,0 +1,81 @@ +/** + * Bridge from the approval service back into the money-moving controllers + * (#314). This is a separate module — rather than importing + * `../controllers/transaction-controller` directly from + * `../approvals/service` — purely to avoid a static circular import: + * transaction-controller.ts imports `guardOperation` from the approval + * service, and the approval service needs to invoke the controller's + * execute* functions once a request crosses its approval threshold. The + * dynamic `import()` here defers loading transaction-controller.ts until + * the payload actually needs executing (long after both modules have + * finished their initial module-load), so neither side has to know about + * the other at load time. + */ +import type { + ExecuteDepositResult, + ExecuteWithdrawResult, +} from '../controllers/transaction-controller' + +export type ApprovalPayload = + | { + type: 'deposit' + userId: string + walletAddress: string + amount: number + assetSymbol: string + memo?: string + actingAsUserId?: string | null + } + | { + type: 'withdraw' + userId: string + walletAddress: string + amount: number + assetSymbol: string + protocolName?: string + memo?: string + actingAsUserId?: string | null + } + +/** + * Re-run an approved payload through the exact same deposit/withdraw path a + * non-approved call would take ("one implementation, two gates"), tagging + * the memo so the resulting Transaction is identifiable as approval-gated + * for audit/tax purposes. `skipApprovalGuard` prevents this execution from + * re-entering `guardOperation` and creating a second approval request for + * the request that just got approved. + */ +export async function runApprovedPayload( + requestId: string, + payload: ApprovalPayload +): Promise { + const { executeDeposit, executeWithdraw } = + await import('../controllers/transaction-controller') + + const memo = payload.memo + ? `${payload.memo} (approval:${requestId})` + : `approval:${requestId}` + + if (payload.type === 'deposit') { + return executeDeposit({ + userId: payload.userId, + walletAddress: payload.walletAddress, + amount: payload.amount, + assetSymbol: payload.assetSymbol, + memo, + actingAsUserId: payload.actingAsUserId, + skipApprovalGuard: true, + }) + } + + return executeWithdraw({ + userId: payload.userId, + walletAddress: payload.walletAddress, + amount: payload.amount, + assetSymbol: payload.assetSymbol, + protocolName: payload.protocolName, + memo, + actingAsUserId: payload.actingAsUserId, + skipApprovalGuard: true, + }) +} diff --git a/src/approvals/service.ts b/src/approvals/service.ts new file mode 100644 index 0000000..1533906 --- /dev/null +++ b/src/approvals/service.ts @@ -0,0 +1,465 @@ +/** + * Approval-workflow service (#314). + * + * A user (or a parent acting on a child via SubAccount delegation) can + * configure an ApprovalPolicy for WITHDRAW/DEPOSIT/MANAGE_STRATEGY that + * requires `minApprovers` independent co-signers before an operation above + * `highValueThreshold` (or every operation, if null) is allowed to execute. + * + * Policy resolution: a delegated operation (actingAsUserId !== userId, i.e. + * a parent acting on a child) resolves the CHILD-scoped policy + * (principalUserId = the parent, scopedToChildUserId = the child); a + * self-operation resolves the OWN-account policy (principalUserId = the + * user, scopedToChildUserId = null). This mirrors how + * src/middleware/subAccount.ts already distinguishes self-access from + * delegated access. + * + * Eligible approvers: there is no separate "approver list" field on + * ApprovalPolicy — the issue's schema doesn't define one. This service uses + * the existing SubAccount graph as the approver pool: the policy's + * `principalUserId` (the parent, or the user themself for a self-policy) + * plus every ACTIVE child under that same parent. That is what lets "the + * parent and a child, or two children" co-sign an operation on a shared + * vault, per the issue's example, without a schema change. + * + * Concurrency: every status transition after PENDING is a conditional + * `updateMany({ where: { status: 'PENDING' } })`, never read-then-write, so + * two simultaneous decisions can't double-execute (see `decide` below). + */ +import { Prisma, SubAccountPermission, ApprovalStatus } from '@prisma/client' +import type { ApprovalPolicy, ApprovalRequest } from '@prisma/client' +import { Decimal } from '@prisma/client/runtime/library' +import db from '../db' +import { logger } from '../utils/logger' +import { AppError } from '../utils/errors' +import { dispatchWebhookEvent } from '../services/webhookDispatcher' +import { getPaginationParams } from '../utils/pagination' +import { runApprovedPayload, type ApprovalPayload } from './executors' + +type Db = typeof db | Prisma.TransactionClient + +function isUniqueConstraintError(err: unknown): boolean { + return ( + err instanceof Prisma.PrismaClientKnownRequestError && err.code === 'P2002' + ) +} + +// ─── Policy resolution ───────────────────────────────────────────────────── + +export async function getActivePolicy( + userId: string, + actingAsUserId: string, + permission: SubAccountPermission, + database: Db = db +): Promise { + if (actingAsUserId === userId) { + return database.approvalPolicy.findFirst({ + where: { + principalUserId: userId, + scopedToChildUserId: null, + permission, + isActive: true, + }, + }) + } + + return database.approvalPolicy.findFirst({ + where: { + principalUserId: actingAsUserId, + scopedToChildUserId: userId, + permission, + isActive: true, + }, + }) +} + +async function getEligibleApproverIds( + policy: Pick, + database: Db +): Promise> { + const ids = new Set([policy.principalUserId]) + const children = await database.subAccount.findMany({ + where: { parentUserId: policy.principalUserId, status: 'ACTIVE' }, + select: { childUserId: true }, + }) + for (const child of children) ids.add(child.childUserId) + return ids +} + +/** True when `userId` is the policy owner or an ACTIVE child of that owner. */ +async function canSeeRequestsForPolicyOwner( + userId: string, + database: Db +): Promise { + const asChild = await database.subAccount.findMany({ + where: { childUserId: userId, status: 'ACTIVE' }, + select: { parentUserId: true }, + }) + return asChild.map((row) => row.parentUserId) +} + +// ─── Guard (entry point from the money path) ────────────────────────────── + +export interface GuardOperationParams { + userId: string // principal whose funds are affected + actingAsUserId?: string | null // who is requesting; defaults to userId + permission: SubAccountPermission + amount: Decimal | number | string + assetSymbol: string + payload: ApprovalPayload + database?: Db +} + +export type GuardResult = + { allowed: true } | { allowed: false; requestId: string; expiresAt: Date } + +/** + * Called from the service layer (executeDeposit/executeWithdraw), never + * from routes directly, so every entry point that reuses those functions is + * covered automatically. Returns `{ allowed: true }` immediately (zero + * friction, unchanged behavior) when no active policy applies or the + * operation is below the policy's threshold. + */ +export async function guardOperation( + params: GuardOperationParams +): Promise { + const database = params.database ?? db + const actingAsUserId = params.actingAsUserId ?? params.userId + const amount = new Decimal(params.amount) + + const policy = await getActivePolicy( + params.userId, + actingAsUserId, + params.permission, + database + ) + if (!policy) return { allowed: true } + + const requiresApproval = + policy.highValueThreshold === null || + amount.greaterThanOrEqualTo(policy.highValueThreshold) + if (!requiresApproval) return { allowed: true } + + // Dedupe: a caller that retries the same logical operation (most notably + // the recurring-deposit job, which re-evaluates a due plan every sweep) + // must land on the same open request rather than piling up duplicates — + // "the requester's intent must never silently vanish" applies just as + // much to not losing track of it under a new id every retry. + const existing = await database.approvalRequest.findFirst({ + where: { + policyId: policy.id, + userId: params.userId, + actingAsUserId, + permission: params.permission, + assetSymbol: params.assetSymbol, + amount, + status: ApprovalStatus.PENDING, + expiresAt: { gt: new Date() }, + }, + }) + if (existing) { + return { + allowed: false, + requestId: existing.id, + expiresAt: existing.expiresAt, + } + } + + const expiresAt = new Date(Date.now() + policy.approvalTimeoutMs) + const request = await database.approvalRequest.create({ + data: { + policyId: policy.id, + userId: params.userId, + actingAsUserId, + permission: params.permission, + amount, + assetSymbol: params.assetSymbol, + payload: params.payload as unknown as Prisma.InputJsonValue, + minApprovers: policy.minApprovers, + expiresAt, + }, + }) + + logger.info('[Approvals] Operation gated pending approval', { + requestId: request.id, + userId: params.userId, + actingAsUserId, + permission: params.permission, + amount: amount.toString(), + assetSymbol: params.assetSymbol, + minApprovers: policy.minApprovers, + }) + + dispatchWebhookEvent('approval.requested', { + requestId: request.id, + userId: params.userId, + actingAsUserId, + permission: params.permission, + amount: amount.toString(), + assetSymbol: params.assetSymbol, + minApprovers: policy.minApprovers, + expiresAt: expiresAt.toISOString(), + }).catch(() => {}) + + return { allowed: false, requestId: request.id, expiresAt } +} + +// ─── Decisions ───────────────────────────────────────────────────────────── + +export interface DecideResult { + status: ApprovalStatus + approvalCount?: number + executionFailed?: boolean +} + +export async function decide( + requestId: string, + approverUserId: string, + approved: boolean, + note: string | undefined, + database: Db = db +): Promise { + const request = await database.approvalRequest.findUnique({ + where: { id: requestId }, + include: { policy: true }, + }) + if (!request) throw new AppError(404, 'Approval request not found') + if (request.status !== ApprovalStatus.PENDING) { + throw new AppError(409, `Request is already ${request.status}`) + } + + const eligible = await getEligibleApproverIds(request.policy, database) + if (!eligible.has(approverUserId)) { + throw new AppError(403, 'Not an eligible approver for this request') + } + + // Self-approval default (issue's documented recommendation): the + // requester cannot count toward their own multi-signature threshold. + if (approverUserId === request.actingAsUserId && request.minApprovers > 1) { + throw new AppError( + 403, + 'Self-approval does not count when more than one approver is required' + ) + } + + if (!approved) { + if (!note) throw new AppError(400, 'A reason is required to reject') + + try { + await database.approval.create({ + data: { requestId, approverUserId, approved: false, note }, + }) + } catch (err) { + if (isUniqueConstraintError(err)) { + throw new AppError( + 409, + 'You have already recorded a decision on this request' + ) + } + throw err + } + + const result = await database.approvalRequest.updateMany({ + where: { id: requestId, status: ApprovalStatus.PENDING }, + data: { status: ApprovalStatus.REJECTED, reason: note }, + }) + if (result.count === 0) { + throw new AppError(409, 'Request is no longer pending') + } + + dispatchWebhookEvent('approval.rejected', { + requestId, + approverUserId, + reason: note, + }).catch(() => {}) + + return { status: ApprovalStatus.REJECTED } + } + + try { + await database.approval.create({ + data: { requestId, approverUserId, approved: true, note }, + }) + } catch (err) { + if (isUniqueConstraintError(err)) { + throw new AppError( + 409, + 'You have already recorded a decision on this request' + ) + } + throw err + } + + dispatchWebhookEvent('approval.approved', { + requestId, + approverUserId, + }).catch(() => {}) + + const approvalCount = await database.approval.count({ + where: { requestId, approved: true }, + }) + if (approvalCount < request.minApprovers) { + return { status: ApprovalStatus.PENDING, approvalCount } + } + + // Threshold crossed. Only the decision that wins this conditional update + // proceeds to execute — a second approval arriving concurrently (or a + // racing expiry sweep) will see count === 0 and stop here. + const claimed = await database.approvalRequest.updateMany({ + where: { id: requestId, status: ApprovalStatus.PENDING }, + data: { status: ApprovalStatus.APPROVED }, + }) + if (claimed.count === 0) { + return { status: ApprovalStatus.PENDING, approvalCount } + } + + return executeApprovedRequest(requestId, database) +} + +async function executeApprovedRequest( + requestId: string, + database: Db +): Promise { + const request = await database.approvalRequest.findUnique({ + where: { id: requestId }, + }) + if (!request) return { status: ApprovalStatus.APPROVED } + + try { + const result = await runApprovedPayload( + requestId, + request.payload as unknown as ApprovalPayload + ) + + if (result.status !== 'CONFIRMED' || !result.transaction) { + // Execution didn't land (on-chain failure). Left APPROVED rather than + // EXECUTED — that's on-chain finality, so this must stay an + // ops-visible stuck state for manual retry/investigation, never a + // silent loss of an approved intent. + logger.error('[Approvals] Approved request failed to execute', { + requestId, + resultStatus: result.status, + }) + return { status: ApprovalStatus.APPROVED, executionFailed: true } + } + + await database.approvalRequest.update({ + where: { id: requestId }, + data: { + status: ApprovalStatus.EXECUTED, + executedAt: new Date(), + executedTxId: result.transaction.id, + }, + }) + + dispatchWebhookEvent('approval.executed', { + requestId, + transactionId: result.transaction.id, + txHash: result.transaction.txHash, + }).catch(() => {}) + + return { status: ApprovalStatus.EXECUTED } + } catch (err) { + const message = err instanceof Error ? err.message : String(err) + logger.error('[Approvals] Execution threw after approval', { + requestId, + error: message, + }) + return { status: ApprovalStatus.APPROVED, executionFailed: true } + } +} + +// ─── Cancellation ────────────────────────────────────────────────────────── + +export async function cancel( + requestId: string, + cancelledById: string, + options: { isAdmin?: boolean } = {}, + database: Db = db +): Promise<{ status: ApprovalStatus }> { + const request = await database.approvalRequest.findUnique({ + where: { id: requestId }, + }) + if (!request) throw new AppError(404, 'Approval request not found') + + if ( + !options.isAdmin && + request.userId !== cancelledById && + request.actingAsUserId !== cancelledById + ) { + throw new AppError(403, 'Only the requester can cancel this request') + } + + const result = await database.approvalRequest.updateMany({ + where: { id: requestId, status: ApprovalStatus.PENDING }, + data: { status: ApprovalStatus.CANCELLED, cancelledById }, + }) + if (result.count === 0) { + throw new AppError(409, `Request is already ${request.status}`) + } + + dispatchWebhookEvent('approval.cancelled', { + requestId, + cancelledById, + }).catch(() => {}) + + return { status: ApprovalStatus.CANCELLED } +} + +// ─── Listing ─────────────────────────────────────────────────────────────── + +export async function listApprovalRequestsForUser( + userId: string, + query: { page?: unknown; limit?: unknown }, + database: Db = db +): Promise<{ + requests: ApprovalRequest[] + page: number + limit: number + total: number +}> { + const { page, limit, skip } = getPaginationParams(query) + const parentIds = await canSeeRequestsForPolicyOwner(userId, database) + + const where: Prisma.ApprovalRequestWhereInput = { + OR: [ + { policy: { principalUserId: userId } }, + ...(parentIds.length > 0 + ? [{ policy: { principalUserId: { in: parentIds } } }] + : []), + ], + } + + const [requests, total] = await Promise.all([ + database.approvalRequest.findMany({ + where, + orderBy: { requestedAt: 'desc' }, + skip, + take: limit, + }), + database.approvalRequest.count({ where }), + ]) + + return { requests, page, limit, total } +} + +export async function getVisibleRequestDetail( + requestId: string, + userId: string, + database: Db = db +): Promise<(ApprovalRequest & { approvals: unknown[] }) | null> { + const request = await database.approvalRequest.findUnique({ + where: { id: requestId }, + include: { policy: true, approvals: true }, + }) + if (!request) return null + + const eligible = await getEligibleApproverIds(request.policy, database) + const visible = + eligible.has(userId) || + request.userId === userId || + request.actingAsUserId === userId + if (!visible) return null + + return request +} diff --git a/src/config/env.ts b/src/config/env.ts index bd97a24..888cde0 100644 --- a/src/config/env.ts +++ b/src/config/env.ts @@ -587,6 +587,11 @@ export const config = { process.env.RECURRING_DEPOSITS_INTERVAL_MS || '300000' ), }, + approvals: { + expirySweepIntervalMs: parseInt( + process.env.APPROVAL_EXPIRY_SWEEP_INTERVAL_MS || '60000' + ), + }, outbox: { dispatchIntervalMs: parseInt( process.env.OUTBOX_DISPATCH_INTERVAL_MS || '15000' diff --git a/src/controllers/transaction-controller.ts b/src/controllers/transaction-controller.ts index cf91f92..3a3cfd8 100644 --- a/src/controllers/transaction-controller.ts +++ b/src/controllers/transaction-controller.ts @@ -10,6 +10,7 @@ import { enqueueOutboxOp } from '../outbox/service' import { dispatchOne } from '../outbox/dispatcher' import { deriveIdempotencyKey } from '../outbox/idempotency' import { OutboxOpKind } from '../outbox/types' +import { guardOperation } from '../approvals/service' /** * Persist the Transaction row (PENDING, no hash yet) and its outbox intent in @@ -108,23 +109,40 @@ export interface ExecuteDepositParams { assetSymbol: string memo?: string actingAsUserId?: string | null + // Set only by src/approvals/executors.ts when re-running an already + // APPROVED request's payload — never by an HTTP route or job directly, or + // an approved request would re-trigger guardOperation and gate itself. + skipApprovalGuard?: boolean } export interface ExecuteDepositResult { - transaction: Transaction - status: 'CONFIRMED' | 'FAILED' + transaction: Transaction | null + status: 'CONFIRMED' | 'FAILED' | 'PENDING_APPROVAL' + approvalRequestId?: string } /** * Core deposit logic extracted for reuse by both the HTTP route and the * recurring deposit scheduler. Submits an on-chain transaction, persists * the Transaction row, and dispatches a webhook on success. + * + * Gated by an ApprovalPolicy (#314) before anything is submitted: this is + * the single interception point, so the HTTP deposit route AND + * src/jobs/recurringDeposits.ts (which calls this function directly) are + * both covered without duplicating the check. */ export async function executeDeposit( params: ExecuteDepositParams ): Promise { - const { userId, walletAddress, amount, assetSymbol, memo, actingAsUserId } = - params + const { + userId, + walletAddress, + amount, + assetSymbol, + memo, + actingAsUserId, + skipApprovalGuard, + } = params const user = await db.user.findUnique({ where: { id: userId }, @@ -134,6 +152,32 @@ export async function executeDeposit( throw new Error('User not found') } + if (!skipApprovalGuard) { + const guard = await guardOperation({ + userId, + actingAsUserId, + permission: 'DEPOSIT', + amount, + assetSymbol, + payload: { + type: 'deposit', + userId, + walletAddress, + amount, + assetSymbol, + memo, + actingAsUserId, + }, + }) + if (!guard.allowed) { + return { + transaction: null, + status: 'PENDING_APPROVAL', + approvalRequestId: guard.requestId, + } + } + } + logger.info('Submitting on-chain deposit', { userId, amount, @@ -180,6 +224,127 @@ export async function executeDeposit( } } +export interface ExecuteWithdrawParams { + userId: string + walletAddress: string + amount: number + assetSymbol: string + protocolName?: string + memo?: string + actingAsUserId?: string | null + // See ExecuteDepositParams.skipApprovalGuard. + skipApprovalGuard?: boolean +} + +export interface ExecuteWithdrawResult { + transaction: Transaction | null + status: 'CONFIRMED' | 'FAILED' | 'PENDING_APPROVAL' + approvalRequestId?: string +} + +/** + * Core withdrawal logic, mirroring executeDeposit. Extracted so both the + * HTTP withdraw route and the approval service's post-approval execution + * path (src/approvals/executors.ts) run through the exact same gate and + * submission logic. + */ +export async function executeWithdraw( + params: ExecuteWithdrawParams +): Promise { + const { + userId, + walletAddress, + amount, + assetSymbol, + protocolName, + memo, + actingAsUserId, + skipApprovalGuard, + } = params + + const user = await db.user.findUnique({ + where: { id: userId }, + select: { id: true, network: true }, + }) + if (!user) { + throw new Error('User not found') + } + + if (!skipApprovalGuard) { + const guard = await guardOperation({ + userId, + actingAsUserId, + permission: 'WITHDRAW', + amount, + assetSymbol, + payload: { + type: 'withdraw', + userId, + walletAddress, + amount, + assetSymbol, + protocolName, + memo, + actingAsUserId, + }, + }) + if (!guard.allowed) { + return { + transaction: null, + status: 'PENDING_APPROVAL', + approvalRequestId: guard.requestId, + } + } + } + + logger.info('Submitting on-chain withdrawal', { + userId, + amount, + assetSymbol, + }) + + const transaction = await enqueueAndDispatch({ + kind: 'WITHDRAW', + userId, + userAddress: walletAddress, + amount, + assetSymbol, + network: user.network, + type: 'WITHDRAWAL', + protocolName, + memo, + actingAsUserId, + }) + + logger.info('On-chain withdrawal completed', { + userId, + txHash: transaction.txHash, + status: transaction.status, + }) + + if (transaction.status === 'CONFIRMED') { + publishUserEvent( + userId, + EVENT_TYPE_TOPIC['transaction.confirmed'], + 'transaction.confirmed', + { + txHash: transaction.txHash, + type: 'WITHDRAWAL', + status: transaction.status, + assetSymbol, + amount, + protocolName, + userId, + } + ).catch(() => {}) + } + + return { + transaction, + status: transaction.status as 'CONFIRMED' | 'FAILED', + } +} + export async function processOnChainTransaction( req: Request, res: Response, @@ -203,33 +368,30 @@ export async function processOnChainTransaction( if (type === 'WITHDRAWAL') { const user = await db.user.findUnique({ where: { id: userId }, - select: { id: true, network: true }, + select: { id: true }, }) if (!user) { return sendNotFound(res, 'User') } - logger.info('Submitting on-chain withdrawal', { - correlationId: req.correlationId, - type, - userId, - amount, - assetSymbol, - }) - - const transaction = await enqueueAndDispatch({ - kind: 'WITHDRAW', + const result = await executeWithdraw({ userId, - userAddress: req.auth!.walletAddress, + walletAddress: req.auth!.walletAddress, amount, assetSymbol, - network: user.network, - type, protocolName, memo, actingAsUserId, }) + if (result.status === 'PENDING_APPROVAL') { + return res.status(202).json({ + status: 'PENDING_APPROVAL', + approvalRequestId: result.approvalRequestId, + }) + } + + const transaction = result.transaction! logger.info('On-chain withdrawal completed', { correlationId: req.correlationId, type, @@ -238,23 +400,8 @@ export async function processOnChainTransaction( status: transaction.status, }) - if (transaction.status === 'CONFIRMED') { - publishUserEvent( - userId, - EVENT_TYPE_TOPIC['transaction.confirmed'], - 'transaction.confirmed', - { - txHash: transaction.txHash, - type, - status: transaction.status, - assetSymbol, - amount, - protocolName, - userId, - } - ).catch(() => {}) - } - + // Notification already dispatched inside executeWithdraw above — do not + // re-publish here (that would double-fire transaction.confirmed). return res.status(201).json({ txHash: transaction.txHash, status: transaction.status, @@ -283,21 +430,29 @@ export async function processOnChainTransaction( actingAsUserId, }) + if (result.status === 'PENDING_APPROVAL') { + return res.status(202).json({ + status: 'PENDING_APPROVAL', + approvalRequestId: result.approvalRequestId, + }) + } + + const transaction = result.transaction! return res.status(201).json({ - txHash: result.transaction.txHash, - status: result.transaction.status, + txHash: transaction.txHash, + status: transaction.status, transaction: { - id: result.transaction.id, - txHash: result.transaction.txHash, - status: result.transaction.status, - amount: Number(result.transaction.amount), - assetSymbol: result.transaction.assetSymbol, - protocolName: result.transaction.protocolName, + id: transaction.id, + txHash: transaction.txHash, + status: transaction.status, + amount: Number(transaction.amount), + assetSymbol: transaction.assetSymbol, + protocolName: transaction.protocolName, }, whatsappReply: formatDepositReply({ - amount: Number(result.transaction.amount), - assetSymbol: result.transaction.assetSymbol, - protocolName: result.transaction.protocolName, + amount: Number(transaction.amount), + assetSymbol: transaction.assetSymbol, + protocolName: transaction.protocolName, }), }) } diff --git a/src/events/types.ts b/src/events/types.ts index a68c153..b7ffd84 100644 --- a/src/events/types.ts +++ b/src/events/types.ts @@ -64,6 +64,15 @@ export const EVENT_TYPE_TOPIC: Record = { 'recurring_deposit.executed': 'transactions', 'recurring_deposit.failed': 'transactions', 'outbox.op_failed': 'transactions', + // #314 — a PENDING_APPROVAL operation's lifecycle is itself a transaction + // state (gating a withdraw/deposit before it submits), so it shares the + // 'transactions' topic rather than introducing a new one. + 'approval.requested': 'transactions', + 'approval.approved': 'transactions', + 'approval.rejected': 'transactions', + 'approval.executed': 'transactions', + 'approval.expired': 'transactions', + 'approval.cancelled': 'transactions', 'agent.rebalanced': 'agent', 'alert_rule.triggered': 'alerts', 'strategy.updated': 'strategies', diff --git a/src/index.ts b/src/index.ts index 2e9bf2f..8c91fce 100644 --- a/src/index.ts +++ b/src/index.ts @@ -56,6 +56,7 @@ import { scheduleAttribution } from './jobs/attribution' import { scheduleOutboxDispatcher } from './outbox/dispatcher' import { scheduleProtocolRiskScoring } from './jobs/protocolRiskScoring' import { schedulePortfolioRiskJob } from './jobs/portfolioRisk' +import { scheduleApprovalExpiry } from './jobs/approvalExpiry' import { startEventListener, stopEventListener } from './stellar/events' import { startEventBridge, stopEventBridge } from './events/bridge' import { attachWebSocketServer, closeWebSocketServer } from './ws/server' @@ -81,6 +82,9 @@ import referralsRouter from './routes/referrals' import recurringDepositRouter from './routes/recurring-deposits' import alertsRouter from './routes/alerts' import strategiesRouter from './routes/strategies' +import subAccountsRouter from './routes/sub-accounts' +import approvalsRouter from './routes/approvals' +import approvalPoliciesRouter from './routes/approval-policies' import keysRouter from './routes/keys' import sessionsRouter from './routes/sessions' import streamRouter from './routes/stream' @@ -122,6 +126,7 @@ let protocolRiskScoringHandle: NodeJS.Timeout | null = null let attributionHandle: NodeJS.Timeout | null = null let outboxDispatcherHandle: NodeJS.Timeout | null = null let portfolioRiskJobHandle: NodeJS.Timeout | null = null +let approvalExpiryHandle: NodeJS.Timeout | null = null function allServicesReady(): boolean { return Object.values(serviceStatus).every((s) => s.ready) @@ -302,6 +307,9 @@ const apiRoutes: ApiRoute[] = [ { path: 'deposit/recurring', handlers: [recurringDepositRouter] }, { path: 'alerts', handlers: [alertsRouter] }, { path: 'strategies', handlers: [strategiesRouter] }, + { path: 'sub-accounts', handlers: [subAccountsRouter] }, + { path: 'approvals', handlers: [approvalsRouter] }, + { path: 'approval-policies', handlers: [approvalPoliciesRouter] }, { path: 'keys', handlers: [keysRouter] }, { path: 'sessions', handlers: [sessionsRouter] }, { path: 'stream', handlers: [streamRouter] }, @@ -417,6 +425,12 @@ async function gracefulShutdown(signal: string): Promise { logger.info('[Shutdown] Portfolio risk job timer cleared') } + if (approvalExpiryHandle) { + clearInterval(approvalExpiryHandle) + approvalExpiryHandle = null + logger.info('[Shutdown] Approval expiry sweep timer cleared') + } + if (!httpServer) { logger.warn('[Shutdown] No HTTP server to close') process.exit(0) @@ -609,6 +623,7 @@ async function main(): Promise { allocationSuggestionsHandle = scheduleAllocationSuggestions() attributionHandle = scheduleAttribution() portfolioRiskJobHandle = schedulePortfolioRiskJob() + approvalExpiryHandle = scheduleApprovalExpiry() } // ── Process-level error guards ──────────────────────────────────────────────── diff --git a/src/jobs/approvalExpiry.ts b/src/jobs/approvalExpiry.ts new file mode 100644 index 0000000..78e565d --- /dev/null +++ b/src/jobs/approvalExpiry.ts @@ -0,0 +1,98 @@ +/** + * Approval expiry sweep (#314). + * + * PENDING ApprovalRequests whose expiresAt has passed are transitioned to + * EXPIRED and a webhook fired, so an operation that never got enough + * co-signers is surfaced rather than left silently dangling. Unlike + * decide()'s single-row conditional update (racing against a concurrent + * approve/reject), this sweep has no other actor contending for the same + * row transition, so a single batched updateMany is safe — it just can't + * race against itself. + */ +import db from '../db' +import { logger, logBackgroundJob } from '../utils/logger' +import { + generateCorrelationId, + runWithCorrelationIdAsync, +} from '../utils/correlation' +import { config } from '../config/env' +import { recordBackgroundJob } from '../utils/metrics' +import { recordJobSuccess, recordJobFailure } from '../utils/job-metrics' +import { dispatchWebhookEvent } from '../services/webhookDispatcher' + +export async function sweepExpiredApprovals(): Promise { + const correlationId = generateCorrelationId() + return runWithCorrelationIdAsync(correlationId, async () => { + const startTime = Date.now() + const jobName = 'approval_expiry_sweep' + + try { + const expired = await db.approvalRequest.findMany({ + where: { status: 'PENDING', expiresAt: { lt: new Date() } }, + select: { id: true, userId: true, actingAsUserId: true }, + }) + + if (expired.length > 0) { + await db.approvalRequest.updateMany({ + where: { + id: { in: expired.map((r) => r.id) }, + status: 'PENDING', + }, + data: { status: 'EXPIRED' }, + }) + + for (const request of expired) { + dispatchWebhookEvent('approval.expired', { + requestId: request.id, + userId: request.userId, + actingAsUserId: request.actingAsUserId, + }).catch(() => {}) + } + } + + const durationMs = Date.now() - startTime + const duration = durationMs / 1000 + + logBackgroundJob(jobName, 'success', duration, correlationId, { + expiredCount: expired.length, + }) + + recordBackgroundJob(jobName, 'success', duration) + recordJobSuccess(jobName, durationMs) + } catch (error) { + const durationMs = Date.now() - startTime + const duration = durationMs / 1000 + const errorMessage = + error instanceof Error ? error.message : 'Unknown error' + + logBackgroundJob(jobName, 'failed', duration, correlationId, { + error: errorMessage, + }) + + recordBackgroundJob(jobName, 'failed', duration) + recordJobFailure(jobName, durationMs) + } + }) +} + +/** + * Schedule the approval expiry sweep to run once at startup, then on a + * fixed interval (default: 60s — approval timeouts are typically much + * shorter-lived than the other cleanup/retention jobs, so this polls more + * frequently). + * + * @returns A NodeJS.Timeout handle (call clearInterval to stop it). + */ +export function scheduleApprovalExpiry(): NodeJS.Timeout { + sweepExpiredApprovals() + + const handle = setInterval( + sweepExpiredApprovals, + config.approvals.expirySweepIntervalMs + ) + + logger.info( + `[ApprovalExpiry] Scheduler started (interval: ${config.approvals.expirySweepIntervalMs}ms)` + ) + return handle +} diff --git a/src/jobs/recurringDeposits.ts b/src/jobs/recurringDeposits.ts index 73865df..1f41f9f 100644 --- a/src/jobs/recurringDeposits.ts +++ b/src/jobs/recurringDeposits.ts @@ -103,7 +103,7 @@ async function executePlan(plan: RecurringDepositPlan): Promise { logger.info('[RecurringDeposit] Plan executed successfully', { planId: plan.id, userId: plan.userId, - txHash: result.transaction.txHash, + txHash: result.transaction!.txHash, }) publishUserEvent( @@ -116,9 +116,26 @@ async function executePlan(plan: RecurringDepositPlan): Promise { amount: Number(plan.amount), assetSymbol: plan.assetSymbol, cadence: plan.cadence, - txHash: result.transaction.txHash, + txHash: result.transaction!.txHash, } ).catch(() => {}) + } else if (result.status === 'PENDING_APPROVAL') { + // Gated by an ApprovalPolicy (#314): skip this occurrence rather than + // executing or failing it. `nextRunAt` is deliberately left untouched + // so the plan is picked up again next sweep — guardOperation's dedupe + // check (same policy/user/amount, still-PENDING) lands on the same + // open request instead of piling up duplicates, so this is a no-op + // poll until an approver decides, not a retry storm. + await db.recurringDepositPlan.update({ + where: { id: plan.id }, + data: { lastRunStatus: 'pending_approval' }, + }) + + logger.info('[RecurringDeposit] Plan occurrence pending approval', { + planId: plan.id, + userId: plan.userId, + approvalRequestId: result.approvalRequestId, + }) } else { await failPlan(plan, 'transaction_failed') } diff --git a/src/middleware/adminAuth.ts b/src/middleware/adminAuth.ts index ef15697..5a72fe3 100644 --- a/src/middleware/adminAuth.ts +++ b/src/middleware/adminAuth.ts @@ -32,6 +32,9 @@ export const ADMIN_SCOPES = [ // cancel unsent PENDING ops). 'outbox:read', 'outbox:write', + // #314 — admin cancellation of a PENDING_APPROVAL request (the issue's + // "requester or admin" cancel rule). + 'approvals:write', 'super', ] as const export type AdminScope = (typeof ADMIN_SCOPES)[number] diff --git a/src/routes/admin.ts b/src/routes/admin.ts index 84c8451..fe8a693 100644 --- a/src/routes/admin.ts +++ b/src/routes/admin.ts @@ -1177,6 +1177,41 @@ router.post( } ) +/** + * POST /api/admin/approvals/:id/cancel + * Admin cancellation of a PENDING_APPROVAL request (#314) — the issue's + * "requester or admin" cancel rule; the requester's own path is + * POST /api/v1/approvals/:id/cancel. Required scope: approvals:write + */ +router.post( + '/approvals/:id/cancel', + requireAdminScope('approvals:write'), + async (req: Request, res: Response) => { + try { + const { cancel } = await import('../approvals/service') + const adminAuth = res.locals.adminAuth + const result = await cancel(req.params.id, adminAuth?.id ?? 'admin', { + isAdmin: true, + }) + auditLog(req, res, 'APPROVAL_ADMIN_CANCEL', 'success', { + requestId: req.params.id, + }) + res.status(200).json({ success: true, data: result }) + } catch (error) { + const statusCode = + error && typeof error === 'object' && 'statusCode' in error + ? (error as { statusCode: number }).statusCode + : 400 + const message = error instanceof Error ? error.message : 'Unknown error' + auditLog(req, res, 'APPROVAL_ADMIN_CANCEL', 'failure', { + requestId: req.params.id, + error: message, + }) + res.status(statusCode).json({ success: false, error: message }) + } + } +) + /** * GET /api/admin/users/:id/sessions — list sessions for a user (#376) */ diff --git a/src/routes/approval-policies.ts b/src/routes/approval-policies.ts new file mode 100644 index 0000000..0ea15a1 --- /dev/null +++ b/src/routes/approval-policies.ts @@ -0,0 +1,161 @@ +import { Router, Request, Response } from 'express' +import { requireAuth } from '../middleware/authenticate' +import { validate } from '../middleware/validate' +import { sendError, sendNotFound } from '../utils/errors' +import { logger } from '../utils/logger' +import db from '../db' +import { + createApprovalPolicySchema, + updateApprovalPolicySchema, +} from '../validators/approval-validators' + +const router = Router() + +/** + * A policy scoped to a child (scopedToChildUserId set) governs a + * sub-account relationship, so creating/editing it requires the caller to + * currently hold MANAGE_STRATEGY on that child — the same authority level + * the issue assigns to sub-account policy changes. Re-checked on every + * write (not just at creation) since the underlying SubAccount grant can be + * revoked or narrowed later. + */ +async function assertManageStrategyOnChild( + parentUserId: string, + childUserId: string, + res: Response +): Promise { + const subAccount = await db.subAccount.findUnique({ + where: { + parentUserId_childUserId: { parentUserId, childUserId }, + }, + }) + if ( + !subAccount || + subAccount.status !== 'ACTIVE' || + !subAccount.permissions.includes('MANAGE_STRATEGY') + ) { + sendError(res, 403, 'Forbidden', { required: 'MANAGE_STRATEGY' }) + return false + } + return true +} + +// ── GET / — policies the caller owns (own-account + any child-scoped) ────── +router.get('/', requireAuth, async (req: Request, res: Response) => { + const policies = await db.approvalPolicy.findMany({ + where: { principalUserId: req.auth!.userId }, + orderBy: { createdAt: 'desc' }, + }) + res.json({ policies }) +}) + +// ── GET /:id ───────────────────────────────────────────────────────────── +router.get('/:id', requireAuth, async (req: Request, res: Response) => { + const policy = await db.approvalPolicy.findUnique({ + where: { id: req.params.id }, + }) + if (!policy || policy.principalUserId !== req.auth!.userId) { + return sendNotFound(res, 'Approval policy') + } + res.json({ policy }) +}) + +// ── POST / — create a policy ──────────────────────────────────────────────── +router.post( + '/', + requireAuth, + validate({ + body: createApprovalPolicySchema, + errorMessage: 'Validation error', + }), + async (req: Request, res: Response) => { + const principalUserId = req.auth!.userId + const { + scopedToChildUserId, + permission, + minApprovers, + highValueThreshold, + approvalTimeoutMs, + } = req.body + + if (scopedToChildUserId) { + const ok = await assertManageStrategyOnChild( + principalUserId, + scopedToChildUserId, + res + ) + if (!ok) return + } + + const policy = await db.approvalPolicy.create({ + data: { + principalUserId, + scopedToChildUserId: scopedToChildUserId ?? null, + permission, + minApprovers, + highValueThreshold: highValueThreshold ?? null, + approvalTimeoutMs, + }, + }) + + logger.info('[ApprovalPolicy] Created', { + principalUserId, + scopedToChildUserId, + permission, + minApprovers, + }) + + res.status(201).json({ policy }) + } +) + +// ── PUT /:id — update a policy ────────────────────────────────────────────── +router.put( + '/:id', + requireAuth, + validate({ + body: updateApprovalPolicySchema, + errorMessage: 'Validation error', + }), + async (req: Request, res: Response) => { + const principalUserId = req.auth!.userId + const policy = await db.approvalPolicy.findUnique({ + where: { id: req.params.id }, + }) + if (!policy) return sendNotFound(res, 'Approval policy') + if (policy.principalUserId !== principalUserId) { + return sendError(res, 403, 'Forbidden') + } + + if (policy.scopedToChildUserId) { + const ok = await assertManageStrategyOnChild( + principalUserId, + policy.scopedToChildUserId, + res + ) + if (!ok) return + } + + const { minApprovers, highValueThreshold, approvalTimeoutMs, isActive } = + req.body + + const updated = await db.approvalPolicy.update({ + where: { id: req.params.id }, + data: { + ...(minApprovers !== undefined && { minApprovers }), + ...(highValueThreshold !== undefined && { highValueThreshold }), + ...(approvalTimeoutMs !== undefined && { approvalTimeoutMs }), + ...(isActive !== undefined && { isActive }), + }, + }) + + logger.info('[ApprovalPolicy] Updated', { + id: req.params.id, + principalUserId, + }) + + res.json({ policy: updated }) + } +) + +export default router diff --git a/src/routes/approvals.ts b/src/routes/approvals.ts new file mode 100644 index 0000000..d1a995c --- /dev/null +++ b/src/routes/approvals.ts @@ -0,0 +1,105 @@ +import { Router, Request, Response } from 'express' +import { requireAuth } from '../middleware/authenticate' +import { validate } from '../middleware/validate' +import { sendError, sendNotFound, AppError } from '../utils/errors' +import { logger } from '../utils/logger' +import { approveSchema, rejectSchema } from '../validators/approval-validators' +import { + decide, + cancel, + listApprovalRequestsForUser, + getVisibleRequestDetail, +} from '../approvals/service' + +const router = Router() + +function handleServiceError(res: Response, err: unknown, action: string) { + if (err instanceof AppError) { + return sendError(res, err.statusCode, err.message) + } + logger.error(`[Approvals] ${action} failed`, { + error: err instanceof Error ? err.message : String(err), + }) + return sendError(res, 500, 'Internal server error') +} + +// ── GET / — requests affecting the caller (as principal or eligible approver) ── +router.get('/', requireAuth, async (req: Request, res: Response) => { + try { + const result = await listApprovalRequestsForUser(req.auth!.userId, { + page: req.query.page, + limit: req.query.limit, + }) + res.json(result) + } catch (err) { + handleServiceError(res, err, 'List') + } +}) + +// ── GET /:id — full request + decisions ───────────────────────────────────── +router.get('/:id', requireAuth, async (req: Request, res: Response) => { + try { + const request = await getVisibleRequestDetail( + req.params.id, + req.auth!.userId + ) + if (!request) { + return sendNotFound(res, 'Approval request') + } + res.json({ request }) + } catch (err) { + handleServiceError(res, err, 'Get') + } +}) + +// ── POST /:id/approve ──────────────────────────────────────────────────────── +router.post( + '/:id/approve', + requireAuth, + validate({ body: approveSchema, errorMessage: 'Validation error' }), + async (req: Request, res: Response) => { + try { + const result = await decide( + req.params.id, + req.auth!.userId, + true, + req.body.note + ) + res.json(result) + } catch (err) { + handleServiceError(res, err, 'Approve') + } + } +) + +// ── POST /:id/reject ───────────────────────────────────────────────────────── +router.post( + '/:id/reject', + requireAuth, + validate({ body: rejectSchema, errorMessage: 'Validation error' }), + async (req: Request, res: Response) => { + try { + const result = await decide( + req.params.id, + req.auth!.userId, + false, + req.body.reason + ) + res.json(result) + } catch (err) { + handleServiceError(res, err, 'Reject') + } + } +) + +// ── POST /:id/cancel — requester (admin cancellation: see routes/admin.ts) ── +router.post('/:id/cancel', requireAuth, async (req: Request, res: Response) => { + try { + const result = await cancel(req.params.id, req.auth!.userId) + res.json(result) + } catch (err) { + handleServiceError(res, err, 'Cancel') + } +}) + +export default router diff --git a/src/validators/approval-validators.ts b/src/validators/approval-validators.ts new file mode 100644 index 0000000..7ca5504 --- /dev/null +++ b/src/validators/approval-validators.ts @@ -0,0 +1,29 @@ +import { z } from 'zod' +import { SubAccountPermission } from '@prisma/client' + +const PERMISSION_VALUES = Object.values(SubAccountPermission) + +export const approveSchema = z.object({ + note: z.string().max(500).optional(), +}) + +// A rejection reason is required (optional on approve) — the issue's +// explicit validation rule. +export const rejectSchema = z.object({ + reason: z.string().min(1, 'A reason is required to reject').max(500), +}) + +export const createApprovalPolicySchema = z.object({ + scopedToChildUserId: z.string().uuid().optional(), + permission: z.enum(PERMISSION_VALUES as [string, ...string[]]), + minApprovers: z.number().int().min(1).max(10), + highValueThreshold: z.number().positive().optional(), + approvalTimeoutMs: z.number().int().positive(), +}) + +export const updateApprovalPolicySchema = z.object({ + minApprovers: z.number().int().min(1).max(10).optional(), + highValueThreshold: z.number().positive().nullable().optional(), + approvalTimeoutMs: z.number().int().positive().optional(), + isActive: z.boolean().optional(), +}) diff --git a/src/validators/webhook-validators.ts b/src/validators/webhook-validators.ts index 52acefb..82a2c23 100644 --- a/src/validators/webhook-validators.ts +++ b/src/validators/webhook-validators.ts @@ -45,6 +45,14 @@ const WEBHOOK_EVENTS = [ // Durable outbox (#325): a money-moving op exhausted its retries and moved // to the terminal FAILED state — see docs/OUTBOX.md. 'outbox.op_failed', + // Approval workflows (#314): lifecycle of a PENDING_APPROVAL high-value + // operation gated by an ApprovalPolicy — see docs/APPROVALS.md. + 'approval.requested', + 'approval.approved', + 'approval.rejected', + 'approval.executed', + 'approval.expired', + 'approval.cancelled', ] as const export const createWebhookSchema = z.object({ diff --git a/tests/unit/approvals/service.test.ts b/tests/unit/approvals/service.test.ts new file mode 100644 index 0000000..c219ef1 --- /dev/null +++ b/tests/unit/approvals/service.test.ts @@ -0,0 +1,426 @@ +// Approval workflow service unit tests (#314). Pin the high-risk invariants: +// * zero-friction path is preserved when no policy applies / below threshold +// * threshold-met creates a PENDING request and dispatches approval.requested +// * repeated identical gated calls dedupe onto the same open request +// * self-approval never counts toward minApprovers > 1 +// * only an eligible approver (policy owner + its ACTIVE children) may decide +// * a race for the threshold-crossing decision executes exactly once +// * cancellation is requester-only unless explicitly called as admin +import { Decimal } from '@prisma/client/runtime/library' +import db from '../../../src/db' +import { dispatchWebhookEvent } from '../../../src/services/webhookDispatcher' +import { runApprovedPayload } from '../../../src/approvals/executors' +import { guardOperation, decide, cancel } from '../../../src/approvals/service' +import { AppError } from '../../../src/utils/errors' + +jest.mock('../../../src/db', () => ({ __esModule: true, default: {} })) +jest.mock('../../../src/services/webhookDispatcher', () => ({ + dispatchWebhookEvent: jest.fn().mockResolvedValue(undefined), +})) +jest.mock('../../../src/approvals/executors', () => ({ + runApprovedPayload: jest.fn(), +})) +jest.mock('../../../src/utils/logger', () => ({ + logger: { + warn: jest.fn(), + error: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + }, +})) + +jest.mock('@prisma/client', () => { + const actual = jest.requireActual('@prisma/client') + return { + ...actual, + Prisma: { + ...actual.Prisma, + PrismaClientKnownRequestError: class extends Error { + code: string + constructor(msg: string, opts: { code: string }) { + super(msg) + this.code = opts.code + } + }, + }, + } +}) + +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { Prisma } = require('@prisma/client') +function uniqueViolation(): Error { + return new Prisma.PrismaClientKnownRequestError('unique', { code: 'P2002' }) +} + +const mockDb = db as any +const mockDispatch = dispatchWebhookEvent as jest.Mock +const mockRunApprovedPayload = runApprovedPayload as jest.Mock + +const PARENT = 'parent-1' +const CHILD = 'child-1' + +const basePolicy = { + id: 'policy-1', + principalUserId: PARENT, + scopedToChildUserId: CHILD, + permission: 'WITHDRAW', + minApprovers: 2, + highValueThreshold: new Decimal(1000), + approvalTimeoutMs: 3_600_000, + isActive: true, +} + +const payload = { + type: 'withdraw' as const, + userId: CHILD, + walletAddress: 'G...CHILD', + amount: 5000, + assetSymbol: 'USDC', +} + +beforeEach(() => { + jest.clearAllMocks() + mockDb.approvalPolicy = { findFirst: jest.fn() } + mockDb.approvalRequest = { + findFirst: jest.fn(), + findUnique: jest.fn(), + create: jest.fn(), + update: jest.fn(), + updateMany: jest.fn(), + findMany: jest.fn(), + count: jest.fn(), + } + mockDb.approval = { + create: jest.fn(), + count: jest.fn(), + } + mockDb.subAccount = { findMany: jest.fn().mockResolvedValue([]) } +}) + +describe('guardOperation', () => { + it('allows the operation when no active policy exists', async () => { + mockDb.approvalPolicy.findFirst.mockResolvedValue(null) + + const result = await guardOperation({ + userId: CHILD, + actingAsUserId: PARENT, + permission: 'WITHDRAW', + amount: 5000, + assetSymbol: 'USDC', + payload, + }) + + expect(result).toEqual({ allowed: true }) + expect(mockDb.approvalRequest.create).not.toHaveBeenCalled() + }) + + it('allows the operation when the amount is below highValueThreshold', async () => { + mockDb.approvalPolicy.findFirst.mockResolvedValue(basePolicy) + + const result = await guardOperation({ + userId: CHILD, + actingAsUserId: PARENT, + permission: 'WITHDRAW', + amount: 500, + assetSymbol: 'USDC', + payload, + }) + + expect(result).toEqual({ allowed: true }) + }) + + it('gates and creates a PENDING request when the threshold is met, dispatching approval.requested', async () => { + mockDb.approvalPolicy.findFirst.mockResolvedValue(basePolicy) + mockDb.approvalRequest.findFirst.mockResolvedValue(null) + mockDb.approvalRequest.create.mockResolvedValue({ + id: 'req-1', + expiresAt: new Date(Date.now() + 3_600_000), + }) + + const result = await guardOperation({ + userId: CHILD, + actingAsUserId: PARENT, + permission: 'WITHDRAW', + amount: 5000, + assetSymbol: 'USDC', + payload, + }) + + expect(result).toEqual({ + allowed: false, + requestId: 'req-1', + expiresAt: expect.any(Date), + }) + const createArg = mockDb.approvalRequest.create.mock.calls[0][0].data + expect(createArg.minApprovers).toBe(2) + expect(createArg.policyId).toBe('policy-1') + expect(mockDispatch).toHaveBeenCalledWith( + 'approval.requested', + expect.objectContaining({ requestId: 'req-1' }) + ) + }) + + it('gates on a null threshold regardless of amount', async () => { + mockDb.approvalPolicy.findFirst.mockResolvedValue({ + ...basePolicy, + highValueThreshold: null, + }) + mockDb.approvalRequest.findFirst.mockResolvedValue(null) + mockDb.approvalRequest.create.mockResolvedValue({ + id: 'req-2', + expiresAt: new Date(), + }) + + const result = await guardOperation({ + userId: CHILD, + actingAsUserId: PARENT, + permission: 'WITHDRAW', + amount: 1, + assetSymbol: 'USDC', + payload, + }) + + expect(result.allowed).toBe(false) + }) + + it('dedupes onto an existing open request instead of creating a duplicate', async () => { + mockDb.approvalPolicy.findFirst.mockResolvedValue(basePolicy) + mockDb.approvalRequest.findFirst.mockResolvedValue({ + id: 'existing-req', + expiresAt: new Date(Date.now() + 1_000_000), + }) + + const result = await guardOperation({ + userId: CHILD, + actingAsUserId: PARENT, + permission: 'WITHDRAW', + amount: 5000, + assetSymbol: 'USDC', + payload, + }) + + expect(result).toEqual({ + allowed: false, + requestId: 'existing-req', + expiresAt: expect.any(Date), + }) + expect(mockDb.approvalRequest.create).not.toHaveBeenCalled() + expect(mockDispatch).not.toHaveBeenCalled() + }) +}) + +describe('decide', () => { + const pendingRequest = { + id: 'req-1', + policyId: 'policy-1', + userId: CHILD, + actingAsUserId: PARENT, + status: 'PENDING', + minApprovers: 2, + policy: basePolicy, + } + + it('rejects a decision from someone outside the approver pool', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue(pendingRequest) + mockDb.subAccount.findMany.mockResolvedValue([]) // PARENT has no ACTIVE children besides CHILD... none returned here + + await expect( + decide('req-1', 'stranger', true, undefined) + ).rejects.toMatchObject({ statusCode: 403 }) + }) + + it('rejects self-approval when minApprovers > 1', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue(pendingRequest) + mockDb.subAccount.findMany.mockResolvedValue([{ childUserId: CHILD }]) + + await expect( + decide('req-1', PARENT, true, undefined) + ).rejects.toMatchObject({ statusCode: 403 }) + }) + + it('requires a reason to reject', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue(pendingRequest) + mockDb.subAccount.findMany.mockResolvedValue([{ childUserId: CHILD }]) + + await expect( + decide('req-1', CHILD, false, undefined) + ).rejects.toMatchObject({ statusCode: 400 }) + }) + + it('rejects with a reason via a conditional PENDING -> REJECTED update', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue(pendingRequest) + mockDb.subAccount.findMany.mockResolvedValue([{ childUserId: CHILD }]) + mockDb.approval.create.mockResolvedValue({}) + mockDb.approvalRequest.updateMany.mockResolvedValue({ count: 1 }) + + const result = await decide('req-1', CHILD, false, 'looks wrong') + + expect(result.status).toBe('REJECTED') + expect(mockDb.approvalRequest.updateMany).toHaveBeenCalledWith({ + where: { id: 'req-1', status: 'PENDING' }, + data: { status: 'REJECTED', reason: 'looks wrong' }, + }) + expect(mockDispatch).toHaveBeenCalledWith( + 'approval.rejected', + expect.objectContaining({ requestId: 'req-1' }) + ) + }) + + it('stays PENDING when approvals are below minApprovers', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue(pendingRequest) + mockDb.subAccount.findMany.mockResolvedValue([{ childUserId: CHILD }]) + mockDb.approval.create.mockResolvedValue({}) + mockDb.approval.count.mockResolvedValue(1) + + const result = await decide('req-1', CHILD, true, undefined) + + expect(result).toEqual({ status: 'PENDING', approvalCount: 1 }) + expect(mockDb.approvalRequest.updateMany).not.toHaveBeenCalled() + expect(mockRunApprovedPayload).not.toHaveBeenCalled() + }) + + it('executes exactly once when the threshold is crossed', async () => { + mockDb.approvalRequest.findUnique + .mockResolvedValueOnce(pendingRequest) // decide()'s initial load + .mockResolvedValueOnce({ ...pendingRequest, payload }) // executeApprovedRequest's re-load + mockDb.subAccount.findMany.mockResolvedValue([{ childUserId: CHILD }]) + mockDb.approval.create.mockResolvedValue({}) + mockDb.approval.count.mockResolvedValue(2) + mockDb.approvalRequest.updateMany.mockResolvedValue({ count: 1 }) + mockDb.approvalRequest.update.mockResolvedValue({}) + mockRunApprovedPayload.mockResolvedValue({ + status: 'CONFIRMED', + transaction: { id: 'tx-1', txHash: '0xabc' }, + }) + + const result = await decide('req-1', CHILD, true, undefined) + + expect(result.status).toBe('EXECUTED') + expect(mockRunApprovedPayload).toHaveBeenCalledTimes(1) + expect(mockDb.approvalRequest.update).toHaveBeenCalledWith({ + where: { id: 'req-1' }, + data: expect.objectContaining({ + status: 'EXECUTED', + executedTxId: 'tx-1', + }), + }) + expect(mockDispatch).toHaveBeenCalledWith( + 'approval.executed', + expect.objectContaining({ requestId: 'req-1', transactionId: 'tx-1' }) + ) + }) + + it('does not execute twice when a concurrent decision already claimed the threshold', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue(pendingRequest) + mockDb.subAccount.findMany.mockResolvedValue([{ childUserId: CHILD }]) + mockDb.approval.create.mockResolvedValue({}) + mockDb.approval.count.mockResolvedValue(2) + // Another decide() call already won the conditional update. + mockDb.approvalRequest.updateMany.mockResolvedValue({ count: 0 }) + + const result = await decide('req-1', CHILD, true, undefined) + + expect(result).toEqual({ status: 'PENDING', approvalCount: 2 }) + expect(mockRunApprovedPayload).not.toHaveBeenCalled() + }) + + it('leaves the request APPROVED (not EXECUTED) when execution fails', async () => { + mockDb.approvalRequest.findUnique + .mockResolvedValueOnce(pendingRequest) + .mockResolvedValueOnce({ ...pendingRequest, payload }) + mockDb.subAccount.findMany.mockResolvedValue([{ childUserId: CHILD }]) + mockDb.approval.create.mockResolvedValue({}) + mockDb.approval.count.mockResolvedValue(2) + mockDb.approvalRequest.updateMany.mockResolvedValue({ count: 1 }) + mockRunApprovedPayload.mockResolvedValue({ + status: 'FAILED', + transaction: null, + }) + + const result = await decide('req-1', CHILD, true, undefined) + + expect(result).toEqual({ status: 'APPROVED', executionFailed: true }) + expect(mockDb.approvalRequest.update).not.toHaveBeenCalled() + }) + + it('rejects a duplicate decision from the same approver with 409', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue(pendingRequest) + mockDb.subAccount.findMany.mockResolvedValue([{ childUserId: CHILD }]) + mockDb.approval.create.mockRejectedValue(uniqueViolation()) + + await expect(decide('req-1', CHILD, true, undefined)).rejects.toMatchObject( + { statusCode: 409 } + ) + }) + + it('rejects a decision on a request that is no longer PENDING', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue({ + ...pendingRequest, + status: 'EXPIRED', + }) + + await expect(decide('req-1', CHILD, true, undefined)).rejects.toMatchObject( + { statusCode: 409 } + ) + }) +}) + +describe('cancel', () => { + const request = { + id: 'req-1', + userId: CHILD, + actingAsUserId: PARENT, + status: 'PENDING', + } + + it('allows the requester to cancel', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue(request) + mockDb.approvalRequest.updateMany.mockResolvedValue({ count: 1 }) + + const result = await cancel('req-1', PARENT) + + expect(result).toEqual({ status: 'CANCELLED' }) + expect(mockDispatch).toHaveBeenCalledWith( + 'approval.cancelled', + expect.objectContaining({ requestId: 'req-1' }) + ) + }) + + it('rejects cancellation from an unrelated user', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue(request) + + await expect(cancel('req-1', 'stranger')).rejects.toMatchObject({ + statusCode: 403, + }) + expect(mockDb.approvalRequest.updateMany).not.toHaveBeenCalled() + }) + + it('allows an admin to cancel regardless of ownership', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue(request) + mockDb.approvalRequest.updateMany.mockResolvedValue({ count: 1 }) + + const result = await cancel('req-1', 'admin-key-1', { isAdmin: true }) + + expect(result).toEqual({ status: 'CANCELLED' }) + }) + + it('rejects cancelling a request that is no longer PENDING', async () => { + mockDb.approvalRequest.findUnique.mockResolvedValue({ + ...request, + status: 'EXECUTED', + }) + mockDb.approvalRequest.updateMany.mockResolvedValue({ count: 0 }) + + await expect(cancel('req-1', PARENT)).rejects.toMatchObject({ + statusCode: 409, + }) + }) +}) + +describe('AppError shape', () => { + it('carries statusCode and message', () => { + const err = new AppError(404, 'not found') + expect(err.statusCode).toBe(404) + expect(err.message).toBe('not found') + }) +}) diff --git a/tests/unit/controllers/transaction-controller.test.ts b/tests/unit/controllers/transaction-controller.test.ts new file mode 100644 index 0000000..6204fa1 --- /dev/null +++ b/tests/unit/controllers/transaction-controller.test.ts @@ -0,0 +1,178 @@ +// Approval-gate wiring in the money path (#314). Pins that guardOperation is +// consulted from INSIDE executeDeposit/executeWithdraw — the single +// interception point the HTTP routes and src/jobs/recurringDeposits.ts all +// share — and that a denied guard short-circuits before any on-chain +// submission or Transaction row is created. skipApprovalGuard (set only by +// src/approvals/executors.ts on the post-approval re-run) bypasses the gate. +process.env.NODE_ENV = 'test' + +import db from '../../../src/db' +import { guardOperation } from '../../../src/approvals/service' +import { enqueueOutboxOp } from '../../../src/outbox/service' +import { dispatchOne } from '../../../src/outbox/dispatcher' +import { + executeDeposit, + executeWithdraw, +} from '../../../src/controllers/transaction-controller' + +jest.mock('../../../src/db', () => ({ + __esModule: true, + default: { + user: { findUnique: jest.fn() }, + transaction: { create: jest.fn(), update: jest.fn() }, + $transaction: jest.fn(), + }, +})) +jest.mock('../../../src/approvals/service', () => ({ + guardOperation: jest.fn(), +})) +jest.mock('../../../src/outbox/service', () => ({ + enqueueOutboxOp: jest.fn(), +})) +jest.mock('../../../src/outbox/dispatcher', () => ({ + dispatchOne: jest.fn(), +})) +jest.mock('../../../src/services/webhookDispatcher', () => ({ + dispatchWebhookEvent: jest.fn().mockResolvedValue(undefined), +})) +jest.mock('../../../src/utils/logger', () => ({ + logger: { info: jest.fn(), error: jest.fn(), warn: jest.fn() }, +})) + +const mockDb = db as any +const mockGuard = guardOperation as jest.Mock +const mockEnqueue = enqueueOutboxOp as jest.Mock +const mockDispatchOne = dispatchOne as jest.Mock + +beforeEach(() => { + jest.clearAllMocks() + mockDb.user.findUnique.mockResolvedValue({ id: 'user-1', network: 'MAINNET' }) +}) + +describe('executeDeposit — approval gate', () => { + it('returns PENDING_APPROVAL and submits nothing when the guard denies', async () => { + mockGuard.mockResolvedValue({ + allowed: false, + requestId: 'req-1', + expiresAt: new Date(), + }) + + const result = await executeDeposit({ + userId: 'user-1', + walletAddress: 'G...WALLET', + amount: 5000, + assetSymbol: 'USDC', + }) + + expect(result).toEqual({ + transaction: null, + status: 'PENDING_APPROVAL', + approvalRequestId: 'req-1', + }) + expect(mockDb.$transaction).not.toHaveBeenCalled() + expect(mockEnqueue).not.toHaveBeenCalled() + expect(mockDispatchOne).not.toHaveBeenCalled() + }) + + it('proceeds to submission when the guard allows', async () => { + mockGuard.mockResolvedValue({ allowed: true }) + mockDb.$transaction.mockImplementation(async (fn: any) => + fn({ + transaction: { + create: jest + .fn() + .mockResolvedValue({ id: 'tx-1', txHash: null, status: 'PENDING' }), + }, + }) + ) + mockEnqueue.mockResolvedValue({ id: 'op-1' }) + mockDispatchOne.mockResolvedValue({ status: 'success', hash: '0xabc' }) + mockDb.transaction.update.mockResolvedValue({ + id: 'tx-1', + txHash: '0xabc', + status: 'CONFIRMED', + }) + + const result = await executeDeposit({ + userId: 'user-1', + walletAddress: 'G...WALLET', + amount: 5000, + assetSymbol: 'USDC', + }) + + expect(result.status).toBe('CONFIRMED') + expect(mockEnqueue).toHaveBeenCalled() + }) + + it('never calls the guard when skipApprovalGuard is set (post-approval re-run)', async () => { + mockDb.$transaction.mockImplementation(async (fn: any) => + fn({ + transaction: { + create: jest + .fn() + .mockResolvedValue({ id: 'tx-2', txHash: null, status: 'PENDING' }), + }, + }) + ) + mockEnqueue.mockResolvedValue({ id: 'op-2' }) + mockDispatchOne.mockResolvedValue({ status: 'success', hash: '0xdef' }) + mockDb.transaction.update.mockResolvedValue({ + id: 'tx-2', + txHash: '0xdef', + status: 'CONFIRMED', + }) + + await executeDeposit({ + userId: 'user-1', + walletAddress: 'G...WALLET', + amount: 5000, + assetSymbol: 'USDC', + skipApprovalGuard: true, + }) + + expect(mockGuard).not.toHaveBeenCalled() + }) +}) + +describe('executeWithdraw — approval gate', () => { + it('returns PENDING_APPROVAL and submits nothing when the guard denies', async () => { + mockGuard.mockResolvedValue({ + allowed: false, + requestId: 'req-2', + expiresAt: new Date(), + }) + + const result = await executeWithdraw({ + userId: 'user-1', + walletAddress: 'G...WALLET', + amount: 5000, + assetSymbol: 'USDC', + }) + + expect(result).toEqual({ + transaction: null, + status: 'PENDING_APPROVAL', + approvalRequestId: 'req-2', + }) + expect(mockDb.$transaction).not.toHaveBeenCalled() + }) + + it('passes permission WITHDRAW to the guard', async () => { + mockGuard.mockResolvedValue({ + allowed: false, + requestId: 'req-3', + expiresAt: new Date(), + }) + + await executeWithdraw({ + userId: 'user-1', + walletAddress: 'G...WALLET', + amount: 5000, + assetSymbol: 'USDC', + }) + + expect(mockGuard).toHaveBeenCalledWith( + expect.objectContaining({ permission: 'WITHDRAW' }) + ) + }) +}) diff --git a/tests/unit/jobs/recurring-deposits-approval.test.ts b/tests/unit/jobs/recurring-deposits-approval.test.ts new file mode 100644 index 0000000..20b49e8 --- /dev/null +++ b/tests/unit/jobs/recurring-deposits-approval.test.ts @@ -0,0 +1,116 @@ +// Recurring deposits x approval workflows (#314): a plan gated by an +// ApprovalPolicy must be skipped, not treated as a failure — no +// recurring_deposit.failed webhook, nextRunAt left untouched so the next +// sweep re-evaluates it (landing on the same open request via +// guardOperation's dedupe, not a new one every tick). +jest.mock('../../../src/utils/logger', () => ({ + logger: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + debug: jest.fn(), + }, + logBackgroundJob: jest.fn(), +})) +jest.mock('../../../src/db', () => ({ + __esModule: true, + default: { + recurringDepositPlan: { + findMany: jest.fn(), + findUnique: jest.fn(), + updateMany: jest.fn(), + update: jest.fn(), + }, + custodialWallet: { findUnique: jest.fn() }, + }, +})) +jest.mock('../../../src/controllers/transaction-controller', () => ({ + executeDeposit: jest.fn(), +})) +jest.mock('../../../src/events/publisher', () => ({ + publishUserEvent: jest.fn().mockResolvedValue(undefined), +})) +jest.mock('../../../src/utils/metrics', () => ({ + recordBackgroundJob: jest.fn(), +})) +jest.mock('../../../src/utils/job-metrics', () => ({ + recordJobSuccess: jest.fn(), + recordJobFailure: jest.fn(), +})) + +import db from '../../../src/db' +import { executeDeposit } from '../../../src/controllers/transaction-controller' +import { publishUserEvent } from '../../../src/events/publisher' +import { processRecurringDeposits } from '../../../src/jobs/recurringDeposits' + +const mockDb = db as any +const mockExecuteDeposit = executeDeposit as jest.Mock +const mockPublish = publishUserEvent as jest.Mock + +const plan = { + id: 'plan-1', + userId: 'user-1', + amount: 5000, + assetSymbol: 'USDC', + cadence: 'WEEKLY', + status: 'ACTIVE', + nextRunAt: new Date(Date.now() - 1000), + lastRunStatus: null, +} + +beforeEach(() => { + jest.clearAllMocks() + mockDb.recurringDepositPlan.findMany.mockResolvedValue([plan]) + mockDb.recurringDepositPlan.findUnique.mockResolvedValue(plan) + mockDb.recurringDepositPlan.updateMany.mockResolvedValue({ count: 1 }) + mockDb.recurringDepositPlan.update.mockResolvedValue({}) + mockDb.custodialWallet.findUnique.mockResolvedValue({ + publicKey: 'G...WALLET', + }) +}) + +it('marks the occurrence pending_approval without advancing nextRunAt or firing a failure webhook', async () => { + mockExecuteDeposit.mockResolvedValue({ + transaction: null, + status: 'PENDING_APPROVAL', + approvalRequestId: 'req-1', + }) + + await processRecurringDeposits() + + const updateCalls = mockDb.recurringDepositPlan.update.mock.calls + const statusUpdate = updateCalls.find( + (call: any[]) => call[0].data.lastRunStatus === 'pending_approval' + ) + expect(statusUpdate).toBeDefined() + expect(statusUpdate[0].data.nextRunAt).toBeUndefined() + + expect(mockPublish).not.toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + 'recurring_deposit.failed', + expect.anything() + ) + expect(mockPublish).not.toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + 'recurring_deposit.executed', + expect.anything() + ) +}) + +it('still executes normally when the guard allows (CONFIRMED)', async () => { + mockExecuteDeposit.mockResolvedValue({ + transaction: { id: 'tx-1', txHash: '0xabc' }, + status: 'CONFIRMED', + }) + + await processRecurringDeposits() + + expect(mockPublish).toHaveBeenCalledWith( + 'user-1', + expect.anything(), + 'recurring_deposit.executed', + expect.objectContaining({ planId: 'plan-1' }) + ) +}) diff --git a/tests/unit/routes/approvals.test.ts b/tests/unit/routes/approvals.test.ts new file mode 100644 index 0000000..b7084c9 --- /dev/null +++ b/tests/unit/routes/approvals.test.ts @@ -0,0 +1,166 @@ +process.env.NODE_ENV = 'test' + +import express from 'express' +import request from 'supertest' +import { Request, Response, NextFunction } from 'express' +import { Network } from '@prisma/client' +import approvalsRouter from '../../../src/routes/approvals' +import { AppError } from '../../../src/utils/errors' +import { + decide, + cancel, + listApprovalRequestsForUser, + getVisibleRequestDetail, +} from '../../../src/approvals/service' + +jest.mock('../../../src/approvals/service', () => ({ + decide: jest.fn(), + cancel: jest.fn(), + listApprovalRequestsForUser: jest.fn(), + getVisibleRequestDetail: jest.fn(), +})) + +jest.mock('../../../src/middleware/authenticate', () => ({ + requireAuth: (req: Request, res: Response, next: NextFunction) => { + if (!req.headers?.authorization) { + res.status(401).json({ error: 'Unauthorized' }) + return + } + req.auth = { + userId: 'user-1', + sessionId: 'session-1', + walletAddress: 'GDZST3XVCDTUJ76ZAV2HA72KYXM4Y5KLTMPQWLBQ3VBLGR4A5YNWHA63', + network: Network.MAINNET, + } + next() + }, +})) + +const app = express() +app.use(express.json()) +app.use('/approvals', approvalsRouter) + +const mockDecide = decide as jest.Mock +const mockCancel = cancel as jest.Mock +const mockList = listApprovalRequestsForUser as jest.Mock +const mockDetail = getVisibleRequestDetail as jest.Mock + +function authHeader() { + return { Authorization: 'Bearer test-token' } +} + +beforeEach(() => { + jest.clearAllMocks() +}) + +describe('GET /approvals', () => { + it('requires auth', async () => { + const res = await request(app).get('/approvals') + expect(res.status).toBe(401) + }) + + it('returns the caller-visible list', async () => { + mockList.mockResolvedValue({ requests: [], page: 1, limit: 5, total: 0 }) + const res = await request(app).get('/approvals').set(authHeader()) + expect(res.status).toBe(200) + expect(mockList).toHaveBeenCalledWith('user-1', expect.any(Object)) + }) +}) + +describe('GET /approvals/:id', () => { + it('returns 404 when the request is not visible to the caller', async () => { + mockDetail.mockResolvedValue(null) + const res = await request(app).get('/approvals/req-1').set(authHeader()) + expect(res.status).toBe(404) + }) + + it('returns the request when visible', async () => { + mockDetail.mockResolvedValue({ id: 'req-1' }) + const res = await request(app).get('/approvals/req-1').set(authHeader()) + expect(res.status).toBe(200) + expect(res.body.request.id).toBe('req-1') + }) +}) + +describe('POST /approvals/:id/approve', () => { + it('approves and returns the service result', async () => { + mockDecide.mockResolvedValue({ status: 'PENDING', approvalCount: 1 }) + const res = await request(app) + .post('/approvals/req-1/approve') + .set(authHeader()) + .send({ note: 'looks fine' }) + expect(res.status).toBe(200) + expect(mockDecide).toHaveBeenCalledWith( + 'req-1', + 'user-1', + true, + 'looks fine' + ) + }) + + it('maps a thrown AppError to its status code', async () => { + mockDecide.mockRejectedValue(new AppError(403, 'Not an eligible approver')) + const res = await request(app) + .post('/approvals/req-1/approve') + .set(authHeader()) + .send({}) + expect(res.status).toBe(403) + expect(res.body.error).toBe('Not an eligible approver') + }) + + it('maps an unexpected error to 500', async () => { + mockDecide.mockRejectedValue(new Error('db exploded')) + const res = await request(app) + .post('/approvals/req-1/approve') + .set(authHeader()) + .send({}) + expect(res.status).toBe(500) + }) +}) + +describe('POST /approvals/:id/reject', () => { + it('rejects a request with no reason (400 from zod validation)', async () => { + const res = await request(app) + .post('/approvals/req-1/reject') + .set(authHeader()) + .send({}) + expect(res.status).toBe(400) + expect(mockDecide).not.toHaveBeenCalled() + }) + + it('rejects a request with a reason', async () => { + mockDecide.mockResolvedValue({ status: 'REJECTED' }) + const res = await request(app) + .post('/approvals/req-1/reject') + .set(authHeader()) + .send({ reason: 'amount looks wrong' }) + expect(res.status).toBe(200) + expect(mockDecide).toHaveBeenCalledWith( + 'req-1', + 'user-1', + false, + 'amount looks wrong' + ) + }) +}) + +describe('POST /approvals/:id/cancel', () => { + it('cancels as the requester', async () => { + mockCancel.mockResolvedValue({ status: 'CANCELLED' }) + const res = await request(app) + .post('/approvals/req-1/cancel') + .set(authHeader()) + expect(res.status).toBe(200) + expect(mockCancel).toHaveBeenCalledWith('req-1', 'user-1') + }) + + it('maps a 409 conflict from an already-decided request', async () => { + mockCancel.mockRejectedValue( + new AppError(409, 'Request is already EXECUTED') + ) + const res = await request(app) + .post('/approvals/req-1/cancel') + .set(authHeader()) + expect(res.status).toBe(409) + }) +})