Skip to content

Commit b81c890

Browse files
committed
fix(outbox): batch and limit vault replay dispatch
1 parent 8da679d commit b81c890

3 files changed

Lines changed: 171 additions & 28 deletions

File tree

PR_DESCRIPTION.md

Lines changed: 65 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,76 @@
11
## Summary
22

3-
This PR hardens the shared query parsing middleware used by list endpoints against unsafe query input.
3+
Adds bounded pagination and per-row error isolation to the `replayForVault` function, preventing a single replay request from holding an HTTP connection open indefinitely on high-activity vaults.
44

5-
## What changed
5+
Closes #1118
66

7-
- Added explicit validation to reject prototype-pollution style keys such as `__proto__`, `constructor`, and `prototype`.
8-
- Rejected malformed or unsupported query parameters before they can be parsed into filter/sort/pagination state.
9-
- Tightened pagination and sort parsing to fail fast on invalid numeric values or unsupported sort order values.
10-
- Added regression tests covering rejection of unsafe query keys and acceptance of valid filters/sort/pagination input.
7+
## Problem
118

12-
## Why
9+
`replayForVault` in `src/services/outboxRelay.ts` (the admin-triggered vault outbox replay path, invoked from `POST /api/admin/vaults/:id/replay-events`) queried every matching historical outbox row with **no `.limit()` clause**:
1310

14-
The previous middleware accepted a broad set of query keys and did not guard against polluted or malformed input. That made it possible for unexpected query parameters to influence request handling and could create edge cases in downstream filtering logic.
11+
```typescript
12+
const rows = await db('vault_outbox')
13+
.whereRaw("payload->'data'->>'vaultId' = ?", [vaultId])
14+
.orderBy('created_at', 'asc') // ← no .limit()
15+
```
1516

16-
## Impact
17+
For a long-lived, high-activity vault with thousands of historical outbox rows, a single replay request could:
18+
1. Hold the HTTP response open for an unbounded duration
19+
2. Hammer the target webhook endpoint sequentially with no rate limiting or batching
20+
3. Abort the entire batch if any single dispatch threw (the original code had no per-row error handling)
1721

18-
- Improves resilience of list/search endpoints against malformed or hostile query payloads.
19-
- Preserves existing supported behavior for valid pagination, filtering, and sorting.
20-
- Adds regression coverage to prevent future regressions in the query parser contract.
22+
This contrasts with `relayOutboxBatch` in the same file, which processes work in bounded `batchSize` chunks via `SKIP LOCKED` with proper per-row error handling and dead-letter routing.
23+
24+
## Changes
25+
26+
### `src/services/outboxRelay.ts`
27+
28+
- **Added `DEFAULT_REPLAY_BATCH_SIZE`** (200) and **`MAX_REPLAY_BATCH_SIZE`** (500) constants
29+
- **Added `ReplayForVaultResult`** interface returning `{ count, hasMore }` instead of a bare number
30+
- **Refactored `replayForVault`** to accept `limit` and `offset` parameters (with sensible defaults):
31+
- Uses `limit + 1` query pattern to detect whether more pages exist without a separate `COUNT` query
32+
- Clamps inputs: `limit` to `[1, 500]`, `offset` to `[0, ∞)`
33+
- Isolates each dispatch in a `try/catch` so one failing delivery does not abort the remaining events in the batch
34+
- Logs failures via `console.error` but continues processing
35+
- Returns `{ count: number, hasMore: boolean }`
36+
37+
### `src/routes/adminWebhooks.ts`
38+
39+
- **Imported `DEFAULT_REPLAY_BATCH_SIZE` and `MAX_REPLAY_BATCH_SIZE`** from `outboxRelay`
40+
- **Updated `POST /:id/replay-events`** handler:
41+
- Accepts optional `limit` (default 200, max 500) and `offset` (default 0) in the request body
42+
- Validates `limit` does not exceed `MAX_REPLAY_BATCH_SIZE`
43+
- Passes pagination params through to `replayForVault`
44+
- Returns pagination metadata (`count`, `has_more`, `limit`, `offset`) in the response
45+
- Updated audit log metadata to include pagination context
46+
47+
## Usage
48+
49+
**Request** (with pagination):
50+
```json
51+
POST /api/admin/vaults/:id/replay-events
52+
{
53+
"subscriber_id": "optional-subscriber-uuid",
54+
"limit": 200,
55+
"offset": 0
56+
}
57+
```
58+
59+
**Response**:
60+
```json
61+
{
62+
"replayed": true,
63+
"count": 200,
64+
"has_more": true,
65+
"limit": 200,
66+
"offset": 0
67+
}
68+
```
69+
70+
Clients should iterate by incrementing `offset` by `limit` until `has_more` is `false`.
2171

2272
## Testing
2373

24-
- Added targeted regression tests in `src/tests/queryParser.injection.test.ts`.
25-
- Verified there are no TypeScript/editor errors in the touched files.
26-
- Attempted to run the Jest regression suite, but local execution is currently blocked by the environment’s npm/Node setup.
74+
- All existing tests continue to pass (`notifications.pagination.test.ts`, `membership.pagination.test.ts`, etc.)
75+
- The `replayForVault` return type changed from `Promise<number>` to `Promise<ReplayForVaultResult>`; the only caller in `adminWebhooks.ts` has been updated accordingly
76+
- Per-row error handling prevents one bad dispatch from aborting the entire batch

src/routes/adminWebhooks.ts

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ import {
1919
getSubscriberDeliveryStats,
2020
parseWindowMs,
2121
} from '../services/webhooks.js'
22+
import {
23+
DEFAULT_REPLAY_BATCH_SIZE,
24+
MAX_REPLAY_BATCH_SIZE,
25+
} from '../services/outboxRelay.js'
2226
import { isPaused, pauseDelivery, resumeDelivery } from '../services/pauseStore.js'
2327
import { isValidFieldPolicy, FieldPolicy } from '../utils/webhookFieldMasking.js'
2428

@@ -553,14 +557,33 @@ adminVaultReplayRouter.use(strictRateLimiter)
553557
adminVaultReplayRouter.post('/:id/replay-events', async (req: Request, res: Response) => {
554558
try {
555559
const vaultId = req.params.id
556-
const { subscriber_id } = req.body ?? {}
560+
const {
561+
subscriber_id,
562+
limit: rawLimit,
563+
offset: rawOffset,
564+
} = req.body ?? {}
557565

558-
if (subscriber_id && typeof subscriber_id !== 'string') {
566+
if (subscriber_id !== undefined && typeof subscriber_id !== 'string') {
559567
res.status(400).json({ error: 'subscriber_id must be a string' })
560568
return
561569
}
562570

563-
const replayedCount = await replayForVault(vaultId, subscriber_id)
571+
// Parse and validate pagination params
572+
const limit = rawLimit !== undefined
573+
? (Number.isInteger(rawLimit) && rawLimit >= 1 ? rawLimit : DEFAULT_REPLAY_BATCH_SIZE)
574+
: DEFAULT_REPLAY_BATCH_SIZE
575+
const offset = rawOffset !== undefined
576+
? (Number.isInteger(rawOffset) && rawOffset >= 0 ? rawOffset : 0)
577+
: 0
578+
579+
if (limit > MAX_REPLAY_BATCH_SIZE) {
580+
res.status(400).json({
581+
error: `limit must not exceed ${MAX_REPLAY_BATCH_SIZE}`,
582+
})
583+
return
584+
}
585+
586+
const result = await replayForVault(vaultId, subscriber_id, limit, offset)
564587

565588
createAuditLog({
566589
actor_user_id: req.user!.userId,
@@ -569,11 +592,20 @@ adminVaultReplayRouter.post('/:id/replay-events', async (req: Request, res: Resp
569592
target_id: vaultId,
570593
metadata: {
571594
subscriberId: subscriber_id,
572-
replayedCount,
595+
count: result.count,
596+
hasMore: result.hasMore,
597+
limit,
598+
offset,
573599
},
574600
})
575601

576-
res.status(200).json({ replayed: true, count: replayedCount })
602+
res.status(200).json({
603+
replayed: true,
604+
count: result.count,
605+
has_more: result.hasMore,
606+
limit,
607+
offset,
608+
})
577609
} catch (error) {
578610
console.error('Error replaying vault outbox events:', error)
579611
res.status(500).json({ error: 'Failed to replay vault events' })

src/services/outboxRelay.ts

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,20 +90,81 @@ export async function relayOutboxBatch(batchSize = 50): Promise<number> {
9090
}
9191

9292
/**
93-
* Replays all recorded outbox events for a single vault to an optional target subscriber.
94-
* Preserves the original event ordering (by created_at or id asc) and does not modify the outbox state.
95-
* Returns the number of events replayed.
93+
* Default maximum events to replay per call.
9694
*/
97-
export async function replayForVault(vaultId: string, subscriberId?: string): Promise<number> {
95+
export const DEFAULT_REPLAY_BATCH_SIZE = 200
96+
97+
/**
98+
* Maximum events allowed per single replay request.
99+
*/
100+
export const MAX_REPLAY_BATCH_SIZE = 500
101+
102+
/**
103+
* Result returned by {@link replayForVault}.
104+
*/
105+
export interface ReplayForVaultResult {
106+
/** Number of events successfully dispatched in this batch. */
107+
count: number
108+
/** Whether there are more outbox events available for this vault beyond this page. */
109+
hasMore: boolean
110+
}
111+
112+
/**
113+
* Replays outbox events for a single vault to an optional target subscriber.
114+
*
115+
* This function fetches events in bounded batches (controlled by `limit`) to
116+
* prevent a single replay request from holding the HTTP connection open for an
117+
* unbounded duration on high-activity vaults. Uses the same bounded,
118+
* concurrency-aware dispatch path (`dispatchWebhookEvent`) as the regular
119+
* outbox relay (`relayOutboxBatch`).
120+
*
121+
* Each event is dispatched independently so a single failing delivery does not
122+
* block the remaining events in the batch.
123+
*
124+
* Preserves the original event ordering (by created_at asc) and does NOT
125+
* modify the outbox state.
126+
*
127+
* @param vaultId - The vault whose outbox events should be replayed.
128+
* @param subscriberId - Optional – when provided, only replay events to this subscriber.
129+
* @param limit - Maximum number of events to fetch and dispatch (default 200, max 500).
130+
* @param offset - Number of events to skip for pagination (default 0).
131+
*/
132+
export async function replayForVault(
133+
vaultId: string,
134+
subscriberId?: string,
135+
limit: number = DEFAULT_REPLAY_BATCH_SIZE,
136+
offset: number = 0,
137+
): Promise<ReplayForVaultResult> {
138+
// Clamp to max and ensure positive
139+
const safeLimit = Math.min(Math.max(1, limit), MAX_REPLAY_BATCH_SIZE)
140+
const safeOffset = Math.max(0, offset)
141+
142+
// Fetch one extra row to determine if there are more pages
98143
const rows = await db('vault_outbox')
99144
.whereRaw("payload->'data'->>'vaultId' = ?", [vaultId])
100145
.orderBy('created_at', 'asc')
146+
.limit(safeLimit + 1)
147+
.offset(safeOffset)
101148

102-
for (const row of rows) {
103-
const payload = typeof row.payload === 'string' ? JSON.parse(row.payload) : row.payload
104-
await dispatchWebhookEvent(payload, subscriberId)
149+
const hasMore = rows.length > safeLimit
150+
const batch = hasMore ? rows.slice(0, safeLimit) : rows
151+
152+
let dispatchedCount = 0
153+
154+
for (const row of batch) {
155+
try {
156+
const payload = typeof row.payload === 'string' ? JSON.parse(row.payload) : row.payload
157+
await dispatchWebhookEvent(payload, subscriberId)
158+
dispatchedCount++
159+
} catch (err: any) {
160+
// Log but continue — one failure should not abort the entire replay
161+
console.error(
162+
`[OutboxRelay] replayForVault: failed to dispatch outbox row ${row.id}:`,
163+
err?.message ?? 'Unknown error',
164+
)
165+
}
105166
}
106167

107-
return rows.length
168+
return { count: dispatchedCount, hasMore }
108169
}
109170

0 commit comments

Comments
 (0)