Skip to content

Commit 6b7fd1a

Browse files
authored
fix(webhooks): stop the generic webhook publishing a closed output schema (#6939)
* fix(webhooks): stop the generic webhook publishing a closed output schema Declaring outputs on the generic webhook trigger did not add three reference completions — it made those three the only legal fields on the block. `collectBlockData` registers any non-empty output declaration as an exhaustive schema, and `resolveBlockReference` then throws `InvalidFieldError` for any reference outside it that resolves to `undefined`. A generic webhook receives whatever the caller sends, so every workflow reading a body field started failing the moment a delivery omitted that field, instead of resolving to `undefined` and letting the condition evaluate falsy as it always had. Revert the declaration to `{}` and record why it has to stay that way. The request metadata is still merged into the workflow input by the provider's `formatInput`; it is only undeclared, which is what keeps the shape open. Offering these as editor completions needs a way to mark outputs as hints rather than a closed schema — a change to `getRegistrySchema`, not to this list. Pins the behavior at the executor level rather than on the trigger config, since the config assertion is what passed while the block was broken. * chore(audits): re-record the workspace module-graph baseline `check:tool-registry-boundary` fails on CI for any branch right now: the knowledge page measures 2255 modules against a 2209 baseline, one module past the max(25, 2%) allowance. It passes locally at 2253, which is why it only shows up in CI — the two platforms resolve a couple of modules differently, and the route happened to sit inside that gap. The drift is not from any one change. 26 of the 34 recorded routes have grown since the baseline was last written, by up to +44. Measuring this branch's route with and without its own diff gives 2253 either way, so it contributes nothing; it is just the branch that happened to cross the line. Re-records all 34 entries, which is what the script prescribes. No gateway was added or removed on any route — only the module counts moved — so the boundary this audit exists to protect is unchanged and the first assertion, that the tool registry stays out of every workspace page graph, still passes. Worth a separate look at why the workspace pages have grown this much; this commit only stops a stale number from blocking unrelated work. * chore(tests): give the block-data test helper an explicit return type
1 parent 00d8a3f commit 6b7fd1a

3 files changed

Lines changed: 106 additions & 58 deletions

File tree

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it, vi } from 'vitest'
5+
import { getBlockSchema } from '@/executor/utils/block-data'
6+
import { resolveBlockReference } from '@/executor/utils/block-reference'
7+
import type { SerializedBlock } from '@/serializer/types'
8+
9+
/**
10+
* These assertions are about what the real block registry publishes, so the global stub — which
11+
* returns one mock block with no outputs — would make every case here pass vacuously.
12+
*/
13+
vi.unmock('@/blocks/registry')
14+
15+
function triggerBlock(type: string, params: Record<string, unknown> = {}): SerializedBlock {
16+
return {
17+
id: 'trigger-1',
18+
metadata: { id: type, name: 'webhook1', category: 'triggers' },
19+
position: { x: 0, y: 0 },
20+
config: { tool: '', params },
21+
inputs: {},
22+
outputs: {},
23+
enabled: true,
24+
} as unknown as SerializedBlock
25+
}
26+
27+
function resolve(
28+
pathParts: string[],
29+
schema: ReturnType<typeof getBlockSchema>
30+
): ReturnType<typeof resolveBlockReference> {
31+
return resolveBlockReference(
32+
'webhook1',
33+
pathParts,
34+
{
35+
blockNameMapping: { webhook1: 'trigger-1' },
36+
blockData: { 'trigger-1': { query: { env: 'prod' } } },
37+
blockOutputSchemas: schema ? { 'trigger-1': schema } : {},
38+
} as never,
39+
{} as never
40+
)
41+
}
42+
43+
describe('generic webhook output schema', () => {
44+
/**
45+
* A generic webhook receives whatever the caller sends, so it must publish no schema at all.
46+
* `collectBlockData` registers any non-empty output declaration as exhaustive, which turns
47+
* every unlisted field into a hard `InvalidFieldError` rather than an absent value.
48+
*/
49+
it('publishes no output schema, leaving the block shape open', () => {
50+
expect(getBlockSchema(triggerBlock('generic_webhook'))).toBeUndefined()
51+
})
52+
53+
it.each([
54+
[{}, 'no flags set'],
55+
[{ acceptOtherMethods: true, exposeRequestHeaders: true }, 'both request-metadata flags on'],
56+
])('stays open with %o (%s)', (params) => {
57+
expect(getBlockSchema(triggerBlock('generic_webhook', params))).toBeUndefined()
58+
})
59+
60+
/**
61+
* The production regression this pins: a Slack interactive payload reaching a workflow that
62+
* reads `actions.0.selected_option.value`. When a delivery omits the field the reference must
63+
* resolve to `undefined` so the condition simply evaluates falsy — not abort the run.
64+
*/
65+
it('resolves an absent body field to undefined instead of throwing', () => {
66+
const schema = getBlockSchema(triggerBlock('generic_webhook'))
67+
68+
expect(() => resolve(['actions', '0', 'selected_option', 'value'], schema)).not.toThrow()
69+
expect(resolve(['actions', '0', 'selected_option', 'value'], schema)?.value).toBeUndefined()
70+
})
71+
72+
it('still resolves request metadata the provider merges into the input', () => {
73+
const schema = getBlockSchema(triggerBlock('generic_webhook'))
74+
75+
expect(resolve(['query', 'env'], schema)?.value).toBe('prod')
76+
})
77+
})

apps/sim/triggers/generic/webhook.test.ts

Lines changed: 16 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@ function setupInstructions(): string {
1313
}
1414

1515
describe('genericWebhookTrigger', () => {
16-
it('declares the request metadata so it can be referenced from later blocks', () => {
17-
expect(Object.keys(genericWebhookTrigger.outputs)).toEqual(['method', 'query', 'headers'])
18-
expect(genericWebhookTrigger.outputs.method.type).toBe('string')
19-
expect(genericWebhookTrigger.outputs.query.type).toBe('object')
20-
expect(genericWebhookTrigger.outputs.headers.type).toBe('object')
16+
/**
17+
* Declaring outputs here does not add editor completions — the executor reads the same list as
18+
* an exhaustive schema and rejects every field outside it. See
19+
* `executor/utils/block-data.test.ts` for the behavior this protects.
20+
*/
21+
it('declares no outputs, because the caller decides the payload shape', () => {
22+
expect(genericWebhookTrigger.outputs).toEqual({})
2123
})
2224

2325
/**
@@ -40,40 +42,21 @@ describe('genericWebhookTrigger', () => {
4042
expect(instructions).toContain('GET, PUT, PATCH and DELETE')
4143
})
4244

43-
it('names every reserved key the input can carry', () => {
44-
const instructions = setupInstructions()
45-
46-
for (const key of Object.keys(genericWebhookTrigger.outputs)) {
47-
expect(instructions).toContain(`"${key}"`)
45+
/**
46+
* Named explicitly rather than derived from `outputs`, which is intentionally empty — deriving
47+
* it would make this assertion vacuous.
48+
*/
49+
it.each(['method', 'query', 'headers'])(
50+
'names the reserved "%s" key the input can carry',
51+
(key) => {
52+
expect(setupInstructions()).toContain(`"${key}"`)
4853
}
49-
})
54+
)
5055

5156
it('names the switch that exposes headers rather than promising them', () => {
5257
expect(setupInstructions()).toContain('"Expose Request Headers"')
5358
})
5459

55-
/**
56-
* Two of the three outputs only exist once a switch is on, so they are conditioned on it: the
57-
* reference dropdown must not offer a field the running webhook will not send.
58-
*/
59-
it.each([
60-
['method', 'acceptOtherMethods'],
61-
['headers', 'exposeRequestHeaders'],
62-
])('gates the %s output on the switch that produces it', (key, field) => {
63-
expect(genericWebhookTrigger.outputs[key].condition).toEqual({
64-
field,
65-
value: [true, 'true'],
66-
})
67-
})
68-
69-
/**
70-
* Query parameters are the one key that is not opt-in, so offering them unconditionally is
71-
* correct — gating them on a switch that does not exist would hide them entirely.
72-
*/
73-
it('offers query unconditionally', () => {
74-
expect(genericWebhookTrigger.outputs.query.condition).toBeUndefined()
75-
})
76-
7760
/**
7861
* Auth is header-based, so a plain link cannot carry it. Saying so is the difference between a
7962
* user disabling auth knowingly and discovering it after publishing an open trigger URL.

apps/sim/triggers/generic/webhook.ts

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -152,33 +152,21 @@ export const genericWebhookTrigger: TriggerConfig = {
152152
],
153153

154154
/**
155-
* Body fields stay undeclared because a generic webhook receives whatever JSON the caller
156-
* sends. The request metadata below is known ahead of time, so it can be offered for reference.
155+
* Deliberately empty, and it must stay that way.
157156
*
158-
* `method` and `headers` are conditioned on the switch that produces them, so the reference
159-
* dropdown never offers a field the running webhook will not send. Both truthy forms are
160-
* matched because a YAML- or Copilot-authored workflow can write the string rather than the
161-
* boolean — the same tolerance `isProviderConfigFlagEnabled` applies at delivery time.
157+
* A generic webhook receives whatever the caller sends, so its output shape is unknowable. The
158+
* executor treats any non-empty output declaration as an exhaustive schema: `collectBlockData`
159+
* registers it, and `resolveBlockReference` then throws `InvalidFieldError` for any reference
160+
* outside it that resolves to `undefined`. Declaring `method`, `query` and `headers` here
161+
* therefore did not add three completions — it made those three the *only* legal fields, and
162+
* every workflow reading a body field failed the moment a delivery omitted it.
163+
*
164+
* The metadata is still merged into the input at delivery time by the generic provider's
165+
* `formatInput`; it is only undeclared, which is what keeps the block's shape open. Offering
166+
* these as editor completions needs a way to mark outputs as hints rather than a closed schema,
167+
* which is a change to `getRegistrySchema`, not to this list.
162168
*/
163-
outputs: {
164-
method: {
165-
type: 'string',
166-
description:
167-
'HTTP method of the request. Yields to a body field of the same name if the caller sends one.',
168-
condition: { field: 'acceptOtherMethods', value: [true, 'true'] },
169-
},
170-
query: {
171-
type: 'object',
172-
description:
173-
'Query parameters from the request URL, when it has any. Yields to a body field of the same name if the caller sends one.',
174-
},
175-
headers: {
176-
type: 'object',
177-
description:
178-
'Request headers, excluding the ones that carry credentials. Yields to a body field of the same name if the caller sends one.',
179-
condition: { field: 'exposeRequestHeaders', value: [true, 'true'] },
180-
},
181-
},
169+
outputs: {},
182170

183171
webhook: {
184172
method: 'POST',

0 commit comments

Comments
 (0)