Skip to content

feat(cdp): let the webhook step sign bodies with hmac-sha256 - #135206

Open
Silthus wants to merge 11 commits into
feat/webhook-body-hmac-signingfrom
feat/webhook-hmac-signing-format
Open

Silthus wants to merge 11 commits into
feat/webhook-body-hmac-signingfrom
feat/webhook-hmac-signing-format

Conversation

@Silthus

@Silthus Silthus commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • Teams whose receiver checks a GitHub-style HMAC-SHA256 of the body can't verify the HTTP webhook step. The step signs only as Standard Webhooks.
  • The need came from a T3 Code receiver. That step is closed, and no other request names this need yet.

Builds on #127117, which taught every worker the new format.

Impact

Watch Webhooks signed with HMAC-SHA256: teams with webhook steps or destinations choose the new format after deploy. Baseline since 2026-11-13.

Note

🔵 Merge risk: low · two-way door until a team builds a receiver on X-Hub-Signature-256

  • Webhook steps and destinations that pick HMAC-SHA256, both regions: a new choice, so only teams that opt in take it.
  • Every webhook with a signing secret and no format keeps Standard Webhooks. Template tests cover the unset, default and new values.
  • Workflow steps load the new template code live, so every webhook step runs it on deploy.
  • Saved webhook destinations show the usual "Modified" tag for a changed template. They get the new choice after updating from the template.
  • Switch-off: none: live on deploy, fix forward.
  • Tier: non-critical

Rollout

  • Stages: merge only after #127117 has deployed in US and EU. An older worker sends an HMAC payload unsigned.
  • Decided: split the template choice into this layer, so the deploy order closes the unsigned window.
  • Watch: no alert pages on this. Watch the webhook step's failed deliveries in the workflow logs.

Changes

  • A new Signature format choice on the webhook step and destination. HMAC-SHA256 of the body sends X-Hub-Signature-256: sha256=<hex> and a webhook-id that stays the same across retries.
  • Standard Webhooks stays the default.
  • The help text says HMAC-SHA256 needs a request body and a secret of at least 24 characters. With GET, the step logs an error and sends nothing.

Webhook step with a signing secret and HMAC-SHA256

How did you test this code?

Test rationale: the webhook template test runs each format through the executor and checks the signature the request carries: unset, Standard Webhooks and HMAC-SHA256. It fails if a format signs the wrong way, or if the queued job holds the secret.

  • Live run through the CDP consumer and Cyclotron worker to a verifying receiver: old stored code, Standard Webhooks and HMAC-SHA256 all verify; retries keep one webhook-id; a wrong secret gets a 401; HMAC-SHA256 with GET sends nothing and logs why.
  • Cross-version: this template's HMAC payload verifies on the fix(cdp): sign the body webhooks send and accept hmac-sha256 signing #127117 executor, and the fix(cdp): sign the body webhooks send and accept hmac-sha256 signing #127117 template's payload verifies here.
  • Prod shape: no stored webhook destination or workflow step sets a format in US or EU, so all keep Standard Webhooks. The counts stay private.
  • UI walk on a production build: the step shows Signature format, saves HMAC-SHA256, and keeps it after a reload. The destination form shows the same field.
  • CI report: the duplication warnings name only files this layer doesn't change. They come from master code that the stacked base doesn't have yet.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5 (claude-opus-5-5)

Skills: /writing-tests, /writing-user-facing-copy, /writing-pr-descriptions, /stacking-prs, takeover, full-stop, impact, merge-risk, pr-description

Security review: not needed

More context for AI agents
  • Session: Claude Code in T3 Code. Split from #127117, which Michael's commits started.
  • Decisions: see the table below.
  • Read closely: the signing choice in webhook.template.ts; a missing signing_format must stay Standard Webhooks.
Question Answer Who Where
Deploy window Split the template choice into this layer, merged after #127117 deploys (replaces "accept unsigned sends") 👍 comment
T3 Code signing Generic webhook option 👍 comment
Header Fixed GitHub style 🤖 comment

Involvement: one design pass, two review panel rounds, two upstream review rounds, three bot review rounds.

🤖 Generated with Claude Code

Adds a signature format choice to the webhook template. HMAC-SHA256 of
the body sends X-Hub-Signature-256 for receivers that check GitHub-style
signatures. Merge only after the executor support below has deployed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

✅ Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

⚠️ Duplication (Python) — 8 new duplicated blocks (worst 246 tokens)

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

First copy Second copy Lines Tokens
posthog/hogql/database/schema/test/test_sessions_v2.py:224 posthog/hogql/database/schema/test/test_sessions_v3.py:332 44 246
posthog/hogql/database/schema/sessions_v1.py:313 posthog/hogql/database/schema/sessions_v2.py:385 34 169
posthog/hogql/database/schema/sessions_v1.py:219 posthog/hogql/database/schema/sessions_v2.py:230 15 106
products/warehouse_sources/sources/secoda/secoda.py:67 products/warehouse_sources/sources/twilio/twilio.py:309 19 101
products/warehouse_sources/sources/bigcommerce/bigcommerce.py:126 products/warehouse_sources/sources/tvmaze/tvmaze.py:99 14 89
products/signals/backend/emission/azure_application_insights_exceptions.py:5 products/signals/backend/emission/mux_errors.py:7 15 87
products/signals/backend/emission/azure_application_insights_exceptions.py:9 products/signals/backend/emission/newrelic_errors.py:11 11 82
products/warehouse_sources/sources/firehydrant/firehydrant.py:99 products/warehouse_sources/sources/helpin/helpin.py:244 13 71
⚠️ Duplication (TypeScript) — 10 new duplicated blocks (worst 264 tokens)

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

First copy Second copy Lines Tokens
products/signals/frontend/inbox/signalSourcesLogic.ts:782 products/signals/frontend/inbox/signalSourcesLogic.ts:909 126 264
products/signals/frontend/inbox/signalSourcesLogic.ts:645 products/signals/frontend/inbox/signalSourcesLogic.ts:782 125 254
frontend/src/products.tsx:1995 products/signals/manifest.tsx:14 13 157
frontend/src/layout/panel-layout/navbar/tabs/navProductsTabLogic.ts:101 frontend/src/scenes/analytics/analyticsPinnedLogic.ts:73 35 113
frontend/src/products.tsx:315 products/signals/manifest.tsx:58 12 96
products/metrics/frontend/suggestedDashboards/QuillSuggestedDashboardsMenu.tsx:28 products/metrics/frontend/suggestedDashboards/SuggestedDashboardsMenu.tsx:64 12 93
products/workflows/frontend/Broadcasts/broadcastWizardLogic.ts:866 products/workflows/frontend/Broadcasts/broadcastWizardLogic.ts:1672 16 73
products/conversations/frontend/components/SupportSetupCard/supportSetupCardLogic.ts:46 products/workflows/frontend/setupGuide/wizard/workflowsOnboardingWizardLogic.ts:181 16 72
products/error_tracking/frontend/scenes/ErrorTrackingIssueScene/errorTrackingIssueSceneLogic.ts:234 products/error_tracking/frontend/scenes/ErrorTrackingIssueScene/errorTrackingIssueSceneLogic.ts:284 16 70
products/error_tracking/frontend/scenes/ErrorTrackingIssueScene/errorTrackingIssueSceneLogic.ts:458 products/error_tracking/frontend/scenes/ErrorTrackingIssueScene/errorTrackingIssueSceneLogic.ts:630 15 70

@posthog posthog Bot 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.

PostHog Review · Standard · reviewed e3b3aa5: no findings.

Silthus and others added 4 commits November 13, 2026 06:49
Run the template's fetch through the executor and check the signature
the receiver gets for each signing format, keeping the check that the
queue payload never holds the secret.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@posthog posthog Bot 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.

PostHog Review · Standard · reviewed fd55330: no findings.

@Silthus
Silthus added this pull request to stack #135445 November 13, 2026 13:23

@posthog posthog Bot 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.

PostHog Review · Standard · reviewed 50acc0e: no findings.

@posthog posthog Bot 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.

PostHog Review · Standard · reviewed 5ae2dad: no findings.

@Silthus
Silthus marked this pull request as ready for review November 13, 2026 15:06
@parameterai

parameterai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Risk: No findings

Follow-up delta adds two fail-closed guards to the HMAC-SHA256 webhook signing path — a 24-byte minimum secret length and a refusal to sign requests with no body — plus test and description copy updates. Both guards stop the request before it is sent and sign exactly the bytes the fetch transmits, so they harden rather than weaken the flow.

Sentinel reviewed a86e53d · Review settings

@stamphog
stamphog Bot requested a review from a team November 13, 2026 15:06
@stamphog

stamphog Bot commented Nov 13, 2026

Copy link
Copy Markdown

Review requests went to the owners of the changed code. These owners were not requested because their part of this change looks trivial:

  • @PostHog/team-ingestion: Your three changes are generated Hog bytecode fixtures for the webhook template, so they add no logic you need to review.

If you were listed here and want to review, you can still assign yourself as a reviewer.

@veria-ai

veria-ai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

PR overview

No security issues were found in this pull request.

Reviews

@posthog-security-review-bot posthog-security-review-bot Bot added the security-review Request a security review label Nov 13, 2026
@posthog-security-review-bot

posthog-security-review-bot Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

✅ Security review complete — 5ae2daddce30

No findings above the confidence threshold, in 5 min.

Add the security-review label to run again on the latest commit.

stamphog[bot]

This comment was marked as outdated.

@posthog-security-review-bot posthog-security-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Agent-driven security review - no findings.

@posthog-security-review-bot posthog-security-review-bot Bot removed the security-review Request a security review label Nov 13, 2026
@stamphog
stamphog Bot dismissed their stale review November 13, 2026 15:30

New commits were pushed — dismissing the stamphog approval from an earlier head. Re-add the trigger label to request a fresh review.

@posthog posthog Bot 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.

PostHog Review · Standard · reviewed 6307e7a: no findings.

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approved.

The security-sensitive changes have current-head independent review and STRONG author familiarity. No unresolved substantive concerns or other refusal grounds were found.

  • Author wrote 79% of the modified lines and has 3 merged PRs in these paths (familiarity STRONG).
  • PostHog Review (Standard) reviewed the current head: no findings.
  • posthog[bot] reviewed the current head.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 25L, 1F substantive, 94L/6F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1c-medium (94L, 6F, single-area, feat)
stamphog 2.8.0 .stamphog/policy.yml @ 6307e7a · reviewed head 6307e7a

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant