Skip to content

Commit 3eb9df8

Browse files
authored
fix(ingest): remove webhook response filtering (#440)
* revert filtering on webhook response * test(ingest): cover cross-address webhook output * fix(ingest): retain webhook response safeguards
1 parent 9d4ce10 commit 3eb9df8

2 files changed

Lines changed: 90 additions & 38 deletions

File tree

packages/ingest/extract/webhook.spec.ts

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
11
import { strict as assert } from 'node:assert'
2-
import { selectValidOutputs, readJsonCapped, MAX_OUTPUTS_PER_VAULT } from './webhook'
2+
import { mq } from 'lib'
3+
import {
4+
MAX_OUTPUT_GROUPS,
5+
MAX_OUTPUTS_PER_ADDRESS,
6+
MAX_TOTAL_OUTPUTS,
7+
readJsonCapped,
8+
selectValidOutputs,
9+
WebhookExtractor
10+
} from './webhook'
311
import type { Data } from './webhook'
412
import type { Output } from 'lib/types'
513
import type { WebhookSubscription } from 'lib/subscriptions'
614

715
const VAULT_A = '0x1111111111111111111111111111111111111111'
8-
const VAULT_B = '0x2222222222222222222222222222222222222222'
916
const OUT_OF_SCOPE = '0x3333333333333333333333333333333333333333'
1017

1118
const subscription: WebhookSubscription = {
@@ -40,35 +47,61 @@ function output(address: string, label = 'apr', chainId = 1): Output {
4047
}
4148

4249
describe('selectValidOutputs', () => {
43-
it('keeps outputs for requested vaults with allowed labels', () => {
44-
const valid = selectValidOutputs([output(VAULT_A), output(VAULT_B)], data([VAULT_A, VAULT_B]))
45-
assert.equal(valid.length, 2)
50+
it('keeps configured-label outputs for an address other than the triggering vault', () => {
51+
assert.deepEqual(selectValidOutputs([output(OUT_OF_SCOPE)], data([VAULT_A])), [output(OUT_OF_SCOPE)])
4652
})
4753

48-
it('drops outputs for vaults that were not requested', () => {
49-
const valid = selectValidOutputs([output(OUT_OF_SCOPE)], data([VAULT_A]))
50-
assert.equal(valid.length, 0)
54+
it('drops outputs for a different chain than requested', () => {
55+
assert.deepEqual(selectValidOutputs([output(OUT_OF_SCOPE, 'apr', 10)], data([VAULT_A])), [])
5156
})
5257

53-
it('drops outputs for a different chain than requested', () => {
54-
const valid = selectValidOutputs([output(VAULT_A, 'apr', 10)], data([VAULT_A], 1))
55-
assert.equal(valid.length, 0)
58+
it('drops a group with an unexpected label', () => {
59+
assert.deepEqual(selectValidOutputs([output(OUT_OF_SCOPE, 'not-allowed')], data([VAULT_A])), [])
60+
})
61+
62+
it('drops a group over the per-address cap', () => {
63+
const many = Array.from({ length: MAX_OUTPUTS_PER_ADDRESS + 1 }, () => output(OUT_OF_SCOPE))
64+
assert.deepEqual(selectValidOutputs(many, data([VAULT_A])), [])
5665
})
5766

58-
it('matches vault addresses case-insensitively', () => {
59-
const valid = selectValidOutputs([output(VAULT_A.toUpperCase().replace('0X', '0x'))], data([VAULT_A.toLowerCase()]))
60-
assert.equal(valid.length, 1)
67+
it('drops a response over the total output cap', () => {
68+
const many = Array.from({ length: MAX_TOTAL_OUTPUTS + 1 }, () => output(OUT_OF_SCOPE))
69+
assert.deepEqual(selectValidOutputs(many, data([VAULT_A])), [])
6170
})
6271

63-
it('drops a vault group with an unexpected label', () => {
64-
const valid = selectValidOutputs([output(VAULT_A, 'not-allowed')], data([VAULT_A]))
65-
assert.equal(valid.length, 0)
72+
it('drops a response over the output group cap', () => {
73+
const many = Array.from({ length: MAX_OUTPUT_GROUPS + 1 }, (_, i) => output(`0x${i.toString(16).padStart(40, '0')}`))
74+
assert.deepEqual(selectValidOutputs(many, data([VAULT_A])), [])
6675
})
76+
})
77+
78+
describe('WebhookExtractor', () => {
79+
it('loads a cross-address output without composition lookup', async () => {
80+
const originalFetch = globalThis.fetch
81+
const originalSecret = process.env.WEBHOOK_SECRET_S_TEST
82+
const responseOutput = {
83+
...output(OUT_OF_SCOPE),
84+
blockNumber: '1',
85+
blockTime: '1'
86+
}
87+
const add = vi.spyOn(mq, 'add').mockResolvedValue({} as never)
88+
globalThis.fetch = vi.fn().mockResolvedValue(new Response(JSON.stringify([responseOutput])))
89+
process.env.WEBHOOK_SECRET_S_TEST = 'secret'
90+
91+
try {
92+
await new WebhookExtractor().extract(data([VAULT_A]))
6793

68-
it('drops a vault group over the per-vault cap', () => {
69-
const many = Array.from({ length: MAX_OUTPUTS_PER_VAULT + 1 }, () => output(VAULT_A))
70-
const valid = selectValidOutputs(many, data([VAULT_A]))
71-
assert.equal(valid.length, 0)
94+
assert.equal(add.mock.calls.length, 1)
95+
assert.deepEqual(add.mock.calls[0], [
96+
mq.job.load.output,
97+
{ batch: [output(OUT_OF_SCOPE)] }
98+
])
99+
} finally {
100+
add.mockRestore()
101+
globalThis.fetch = originalFetch
102+
if (originalSecret === undefined) delete process.env.WEBHOOK_SECRET_S_TEST
103+
else process.env.WEBHOOK_SECRET_S_TEST = originalSecret
104+
}
72105
})
73106
})
74107

packages/ingest/extract/webhook.ts

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import { z } from 'zod'
21
import { createHmac } from 'crypto'
32
import { mq, sentry } from 'lib'
4-
import { Output, OutputSchema, zhexstring } from 'lib/types'
53
import { WebhookSubscription, WebhookSubscriptionSchema } from 'lib/subscriptions'
4+
import { Output, OutputSchema, zhexstring } from 'lib/types'
5+
import { z } from 'zod'
66

77
export const DataSchema = z.object({
88
abiPath: z.string(),
@@ -81,33 +81,53 @@ export async function readJsonCapped(response: Response, maxBytes = MAX_RESPONSE
8181
return JSON.parse(Buffer.concat(chunks).toString('utf8'))
8282
}
8383

84-
export const MAX_OUTPUTS_PER_VAULT = 100
84+
export const MAX_OUTPUTS_PER_ADDRESS = 100
85+
export const MAX_OUTPUT_GROUPS = 1_000
86+
export const MAX_TOTAL_OUTPUTS = 10_000
8587

86-
// Drop any receiver output that isn't for the chain/vaults we asked it to compute,
87-
// exceeds the per-vault cap, or carries an unexpected label, before it reaches the
88-
// load queue. Scope filtering also bounds total groups to the requested vault set
89-
// (FINDINGS.md findings 4-5, CWE-20 / CWE-400).
88+
// A configured webhook is a trusted producer for its configured labels. It may
89+
// publish for any address on the requested chain, but each response remains
90+
// bounded before it reaches the load queue.
9091
export function selectValidOutputs(outputs: Output[], data: Data): Output[] {
9192
const { subscription } = data
92-
const requestedVaults = new Set(data.vaults.map(v => v.toLowerCase()))
93-
const grouped = Map.groupBy(outputs, o => `${o.chainId}:${o.address}`)
93+
if (outputs.length > MAX_TOTAL_OUTPUTS) {
94+
console.error(`🤬 ${subscription.id} skipping response: ${outputs.length} outputs > ${MAX_TOTAL_OUTPUTS}`)
95+
sentry.captureMessage('WEBHOOK_TOTAL_OUTPUTS_OVER_LIMIT', {
96+
level: 'warning',
97+
tags: { component: 'ingest', job: 'extract.webhook' },
98+
extra: { subscriptionId: subscription.id, outputs: outputs.length, max: MAX_TOTAL_OUTPUTS }
99+
})
100+
return []
101+
}
102+
103+
const grouped = Map.groupBy(outputs, o => `${o.chainId}:${o.address.toLowerCase()}`)
104+
if (grouped.size > MAX_OUTPUT_GROUPS) {
105+
console.error(`🤬 ${subscription.id} skipping response: ${grouped.size} output groups > ${MAX_OUTPUT_GROUPS}`)
106+
sentry.captureMessage('WEBHOOK_OUTPUT_GROUPS_OVER_LIMIT', {
107+
level: 'warning',
108+
tags: { component: 'ingest', job: 'extract.webhook' },
109+
extra: { subscriptionId: subscription.id, groups: grouped.size, max: MAX_OUTPUT_GROUPS }
110+
})
111+
return []
112+
}
113+
94114
return [...grouped].flatMap(([key, group]) => {
95115
const [first] = group
96-
if (first.chainId !== data.chainId || !requestedVaults.has(first.address.toLowerCase())) {
97-
console.error(`🤬 ${subscription.id} skipping ${key}: out of requested scope`)
98-
sentry.captureMessage('WEBHOOK_OUT_OF_SCOPE', {
116+
if (first.chainId !== data.chainId) {
117+
console.error(`🤬 ${subscription.id} skipping ${key}: unexpected chain`)
118+
sentry.captureMessage('WEBHOOK_UNEXPECTED_CHAIN', {
99119
level: 'warning',
100120
tags: { component: 'ingest', job: 'extract.webhook' },
101121
extra: { subscriptionId: subscription.id, key, requestedChainId: data.chainId }
102122
})
103123
return []
104124
}
105-
if (group.length > MAX_OUTPUTS_PER_VAULT) {
106-
console.error(`🤬 ${subscription.id} skipping ${key}: ${group.length} outputs > ${MAX_OUTPUTS_PER_VAULT}`)
125+
if (group.length > MAX_OUTPUTS_PER_ADDRESS) {
126+
console.error(`🤬 ${subscription.id} skipping ${key}: ${group.length} outputs > ${MAX_OUTPUTS_PER_ADDRESS}`)
107127
sentry.captureMessage('WEBHOOK_OUTPUTS_OVER_LIMIT', {
108128
level: 'warning',
109129
tags: { component: 'ingest', job: 'extract.webhook' },
110-
extra: { subscriptionId: subscription.id, key, outputs: group.length, max: MAX_OUTPUTS_PER_VAULT }
130+
extra: { subscriptionId: subscription.id, key, outputs: group.length, max: MAX_OUTPUTS_PER_ADDRESS }
111131
})
112132
return []
113133
}
@@ -144,9 +164,8 @@ export class WebhookExtractor {
144164

145165
const body = await readJsonCapped(response)
146166
const outputs = OutputSchema.array().parse(body)
147-
const valid = selectValidOutputs(outputs, data)
148167

149-
await mq.add(mq.job.load.output, { batch: valid })
168+
await mq.add(mq.job.load.output, { batch: selectValidOutputs(outputs, data) })
150169
} finally {
151170
semaphore.release()
152171
}

0 commit comments

Comments
 (0)