Skip to content

Add inbound webhook ingestion for connectors (Stripe first) - #1372

Merged
helpcodeai merged 7 commits into
HelpCode-ai:mainfrom
rangarajan19:feat/webhook-ingestion
Nov 1, 2026
Merged

helpcodeai merged 7 commits into
HelpCode-ai:mainfrom
rangarajan19:feat/webhook-ingestion

Conversation

@rangarajan19

Copy link
Copy Markdown
Contributor

Summary

@helpcodeai First slice of inbound webhook ingestion, as discussed and agreed on in #988: the POST /api/webhooks/:connectorId endpoint with Stripe signature verification, the <prefix>_recent_events builtin tool, the WebhookEvent model, and retention. Scoped to Stripe only, per your note to leave the generic HMAC fallback and other providers for follow-ups.

If anything here is off, or there's a better way to do a part of it, please just say so — happy to fix it right away.

Changes

  • WebhookEvent Prisma model + 3 new nullable columns on Connector (webhookEnabled, webhookProvider, webhookSecret); webhookSecret is encrypted with the same helpers as authConfig and never returned by any API or tool response (verified against every connector-serializing path, including the export/import util).
  • POST /api/webhooks/:connectorId (webhook-ingest.controller.ts), outside ee/: Stripe-Signature verification (HMAC-SHA256 over timestamp.body, 300s replay tolerance), a body cap below the global 10MB limit checked before the HMAC runs, @Throttle() on the route, and an identical response for an unknown connector id, a disabled hook, and a bad signature.
  • main.ts's existing raw-body capture (previously only /api/support/webhook) now also covers /api/webhooks/, since the HMAC needs the exact bytes sent, not a re-serialized body.
  • <prefix>_recent_events builtin tool, auto-created the same way the existing GraphQL/Email builtins are. It reads WebhookEvent through a new special case in DynamicMcpTools.executeWithEngine() (same pattern as the existing static/email/odata_* cases), since it's a read of our own storage, never an outbound call. Added the matching read-only special case in tool-annotations.ts too, which has its own independent method-name dispatch and would otherwise have left the tool's readOnlyHint unset.
  • Retention: a periodic sweep (shaped like ConnectorAuthStatusService's) plus a per-connector cap enforced on insert. Both configurable via WEBHOOK_EVENT_RETENTION_DAYS (default 7) and WEBHOOK_EVENT_MAX_PER_CONNECTOR (default 500), documented in docs/operations/observability.md.
  • Fixed 6 pre-existing spec files whose DynamicMcpTools construction used positional arguments that silently shifted out of alignment once the new required constructor parameter was added; one of them (code-mode.integration.spec.ts) would have changed what it actually tested, not just failed to type-check.

Type

  • New feature

Testing

  • Added/updated unit tests
  • Tested manually (described below)

Commands run from packages/backend unless noted:

  • npx tsc --noEmit -p tsconfig.json — 0 errors (confirmed against a clean baseline via git stash -u diff; no new errors introduced).
  • npm run lint — 0 errors; 11 pre-existing warnings in unrelated files, unchanged.
  • npx jest src/connectors/webhook-signature.util.spec.ts — 9/9 (valid signature, wrong secret, tampered body, stale timestamp, malformed/missing header, non-numeric timestamp, length-mismatch guard, empty secret).
  • npx jest src/connectors/webhook-events.service.spec.ts — 9/9 (record + dedupe on redelivery, cap enforcement across connectors, query scoping, retention sweep).
  • npx jest src/connectors/webhook-ingest.controller.spec.ts — 10/10 (valid delivery, unknown connector, disabled hook, webhooks-not-enabled, wrong provider, bad signature, missing/oversized body, duplicate delivery, non-JSON body).
  • npx jest src/mcp-server/dynamic-mcp-tools.webhook.spec.ts — 3/3, exercising the actual tool-dispatch wiring in DynamicMcpTools.executeTool(), not just the service in isolation.
  • npx jest src/mcp-server/tool-annotations.spec.ts — 44/44, including the new read-only case for webhook_recent_events.
  • npx jest src/connectors src/mcp-server src/health src/common --maxWorkers=2 — 159/163 suites, 3012/3029 tests passed (4 skipped suites, 17 skipped tests, all pre-existing and unrelated).
  • node scripts/generate-llms-txt.mjs --check (repo root) — exit 0.

Manual: ran the real backend against a real Postgres instance, through the actual HTTP API (not direct DB writes) — logged in, created a connector with webhookEnabled/webhookProvider: stripe/webhookSecret via POST /api/connectors and confirmed webhookSecret is absent from the response, confirmed the builtin tool was auto-created, called GET .../tools/:toolId/annotations and confirmed effective.readOnlyHint: true, and sent real Stripe-signed deliveries to POST /api/webhooks/:connectorId — valid (200, stored correctly), bad signature (401), unknown connector (401, identical to bad signature), missing signature (401), wrong method (404), and an exact replay of the same event id (200, no error, confirmed via direct DB query that only one row exists). All dry-run rows were cleaned up afterward.

Checklist

  • My code follows the existing code style
  • I have tested my changes locally
  • I have updated documentation (docs/operations/observability.md, regenerated llms-full.txt)
  • All existing tests pass
  • This PR has a descriptive title

Related Issues

Relates to #988. No separate issue was filed for this slice: the scope (Stripe-only, this endpoint + this tool) was agreed on directly in the #988 discussion, where it was assigned to me.

Cloud vs self-hosted

Same code path on both; no branch anywhere in this PR checks DEPLOYMENT_MODE. Ingestion, verification, storage, and retention all behave identically.

Known follow-ups (left out of this PR on purpose)

  • Other providers (Meta/WhatsApp) and the generic HMAC fallback, each needing their own verification scheme — flagged in Webhook ingestion: register a webhook as an MCP resource #988 as separate work.
  • The connector export/import format doesn't yet include the three new webhook columns, so exporting a webhook-enabled connector currently loses that config on re-import. Not a security issue (no secret leak either way — it's an explicit allowlist that simply doesn't name these fields yet), just an incompleteness I noticed while checking that webhookSecret can't leak through that path. Happy to add it here or as a follow-up, whichever you'd prefer.

Any connector can opt in (webhookEnabled/webhookProvider/webhookSecret)
to receive inbound webhooks at POST /api/webhooks/:connectorId,
independent of its own connector type. Only Stripe's signature scheme
is verified for now, per the scope agreed in issue HelpCode-ai#988: a generic
HMAC fallback and other providers (Meta/WhatsApp) are left for
follow-up PRs so this one stays small and reviewable.

- WebhookEvent model stores verified deliveries only; nothing lands
  there before the signature checks out. A periodic sweep plus a
  per-connector cap on insert bound storage, both env-configurable
  (WEBHOOK_EVENT_RETENTION_DAYS, WEBHOOK_EVENT_MAX_PER_CONNECTOR).
- webhookSecret is encrypted with the same helpers as authConfig and
  never returned by any API or tool response.
- The endpoint answers identically for an unknown connector id, a
  disabled hook, and a bad signature, so it is not an oracle for
  probing which connector ids exist. Body size is capped below the
  global 10MB limit, checked before the HMAC runs. main.ts's existing
  raw-body capture (previously only /api/support/webhook) now also
  covers /api/webhooks/, since Stripe's HMAC needs the exact bytes
  sent, not a re-serialized body.
- Connectors that opt in get one builtin tool, <prefix>_recent_events,
  auto-created the same way the existing GraphQL/Email builtins are.
  It reads WebhookEvent through a new special case in
  DynamicMcpTools.executeWithEngine() (same pattern as the existing
  `static`/email/odata_* cases) rather than any engine, since it never
  makes an outbound call. Also added the matching read-only special
  case in tool-annotations.ts, which has its own independent
  method-name dispatch and would otherwise have left the tool's
  readOnlyHint unset.
- Fixed 6 pre-existing spec files whose DynamicMcpTools construction
  used positional arguments that silently shifted out of alignment
  once a new required constructor parameter was added; one of them
  (code-mode.integration.spec.ts) would have changed what it actually
  tested, not just failed to type-check.

Verified live against a real Postgres instance and the real HTTP API
(login, connector creation, builtin tool auto-creation, annotations,
and the webhook endpoint itself with actually-computed signatures),
not only unit tests.

@helpcodeai helpcodeai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is solid work and it follows what we agreed in #988. The signature check, the identical 401 answers, the encrypted secret and the organizationId filter in recentEvents all look right, and the migration is additive.

Three changes before we merge:

  1. Tool on update. The <prefix>_recent_events tool is only created in create(). If someone turns webhookEnabled on later with PUT /connectors/:id, no tool appears, and if they turn it off the tool stays. Please create the tool when the flag goes from false to true on update, and remove it when it goes from true to false. A spec for both cases would be great.
  2. limit input. params.limit as number is passed as is. A string like "abc" gives NaN, and Math.min(Math.max(NaN, 1), 200) is NaN, so Prisma fails on take: NaN. Please convert with Number() and fall back to 20 when the result is not a finite number.
  3. Size and personal data. One call can return 200 events of up to 256 KB each. That is up to 50 MB of raw Stripe payloads in the model context, and Stripe events carry customer names, e-mails and addresses. Please lower the maximum to 50 and say in docs/operations/observability.md (or the connector docs) that payloads are stored for 7 days and returned to the model as they are.

On our side we still need to decide about plan limits on Cloud and a dashboard UI. Neither blocks this PR: the API-only version is fine as a first slice.

…ut, event cap

Per @helpcodeai's review on PR HelpCode-ai#1372 (issue HelpCode-ai#988):

- Create/remove the <prefix>_recent_events builtin tool when
  webhookEnabled flips on/off through PUT /connectors/:id, not only at
  connector creation. Matched by endpointMapping.method on removal, so
  a rename in the same request can't leave the old tool behind.
- Coerce recentEvents()'s limit with Number() and fall back to 20 when
  it is not a finite number, instead of letting a non-numeric value
  (e.g. "abc") reach Prisma as take: NaN.
- Lower the per-call event cap from 200 to 50 (a Stripe payload can be
  up to 256KB, carrying customer PII) and document in
  docs/operations/observability.md that stored payloads are returned
  to the model as received and kept for WEBHOOK_EVENT_RETENTION_DAYS.

Added specs for both tool-lifecycle transitions and the limit
coercion; regenerated llms-full.txt for the doc change.
@rangarajan19

Copy link
Copy Markdown
Contributor Author

@helpcodeai I've pushed an update addressing all three points from your review:

  1. <prefix>_recent_events is now created/removed when webhookEnabled flips on/off through PUT /connectors/:id, matched by endpointMapping.method so a rename in the same request can't leave a stale tool behind. Added specs for both transitions.
  2. recentEvents()'s limit is now coerced with Number(), falling back to 20 when it's not a finite number.
  3. Lowered the per-call cap to 50 and documented in docs/operations/observability.md that stored payloads are returned to the model as received and kept for WEBHOOK_EVENT_RETENTION_DAYS.

Let me know if anything's not quite right — happy to adjust.

@helpcodeai helpcodeai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, all three points are fixed: the tool follows webhookEnabled on update (and matching by endpointMapping.method is a good call), limit falls back to 20, and the cap is 50 with the retention documented. Merging now. Plan limits on Cloud and a dashboard UI are on our side, we will open follow-up issues for them.

@helpcodeai
helpcodeai enabled auto-merge (squash) November 1, 2026 09:58
@helpcodeai

Copy link
Copy Markdown
Contributor

@rangarajan19 main got a new spec (trial-blocked-call.spec.ts, from #1390) that builds DynamicMcpTools with positional arguments, so the new WebhookEventsService parameter broke the typecheck after the branch update. I pushed a one-line fix to your branch. Nothing for you to do.

@helpcodeai
helpcodeai merged commit 99e3934 into HelpCode-ai:main Nov 1, 2026
14 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 1, 2026
@rangarajan19
rangarajan19 deleted the feat/webhook-ingestion branch November 7, 2026 05:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants