Skip to content

fix: require Segment identify webhook secret#33

Open
BunsDev wants to merge 1 commit into
mainfrom
codex/fix-unsigned-segment-identify-webhook-vulnerability
Open

fix: require Segment identify webhook secret#33
BunsDev wants to merge 1 commit into
mainfrom
codex/fix-unsigned-segment-identify-webhook-vulnerability

Conversation

@BunsDev
Copy link
Copy Markdown
Member

@BunsDev BunsDev commented Jun 5, 2026

Motivation

  • Prevent unauthenticated Segment identify webhooks from mutating user metadata when an integration has no incomingSecret configured, which allowed attackers to overwrite attributes by supplying forged identifies.

Description

  • Require a non-empty incomingSecret in segmentUserSync.handleIdentify and return 401 when missing to block unsigned requests before parsing the body.
  • Always validate the x-signature HMAC-SHA1 header using createHmac and timingSafeEqual before trusting payload fields.
  • Add regression tests for unsigned/no-secret rejection and valid-signed acceptance in apps/web/src/lib/server/integrations/segment/__tests__/user-sync.test.ts.
  • Preserve existing behavior for outbound segment syncs and keep error handling unchanged.

Testing

  • Ran formatting with bunx prettier --write and verified git diff --check succeeded.
  • Added unit tests in apps/web/src/lib/server/integrations/segment/__tests__/user-sync.test.ts, but running vitest in this environment was blocked due to registry/dependency resolution errors (npm returned 403) so tests could not be executed here.
  • Committed the change as fix: require Segment identify webhook secret and recommend running CI (bun run test) to execute the new tests in the normal developer environment or CI where dependencies are available.

Codex Task

Copilot AI review requested due to automatic review settings June 5, 2026 23:57
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the inbound Segment identify webhook path to prevent unauthenticated requests from mutating user metadata, specifically when the integration lacks an incomingSecret.

Changes:

  • Require a non-empty incomingSecret for inbound Segment identify handling and return 401 when missing.
  • Always enforce x-signature HMAC-SHA1 validation (using timingSafeEqual) before JSON parsing / trusting payload fields.
  • Add unit tests covering missing-secret rejection and valid signed acceptance for segmentUserSync.handleIdentify.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
apps/web/src/lib/server/integrations/segment/user-sync.ts Enforces mandatory incomingSecret and signature verification for inbound identify requests.
apps/web/src/lib/server/integrations/segment/tests/user-sync.test.ts Adds regression tests for the inbound identify signature/secret requirements.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@BunsDev BunsDev self-assigned this Jun 6, 2026
@BunsDev BunsDev force-pushed the codex/fix-unsigned-segment-identify-webhook-vulnerability branch from ffe197e to 4d3ea8d Compare June 6, 2026 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants