mirror of https://github.com/hanzo-fi/webhooks.git synced 2026-10-30 20:32:55 +00:00

Files

T

Maxence MaireauxandGitHub 6504cd9692 fix: prevent webhook message loss, remove panic on unknown filter, stop logging secrets (#142 )

* fix: resolve P0/P1 reliability and security issues

- P0: Process messages synchronously to prevent data loss on crash
  The Watermill handler now blocks until webhook delivery completes.
  Messages are only acknowledged after processing, not before.
  Errors on invalid payloads are returned (nack) instead of swallowed.
  Per-config failures use continue instead of return, so one bad
  endpoint no longer blocks delivery to other matching configs.

- P1: Replace panic with error on unknown filter key
  FindManyConfigs now returns an error instead of panicking when
  called with an unrecognized filter key. A single bad API request
  no longer crashes the process.

- P1: Remove environment and secret logging
  Removed os.Environ() dumps from server and worker startup.
  Replaced config %+v logging with safe ID/endpoint-only logging.
  Removed event-payload from OpenTelemetry span attributes.

- Docs: Add architecture, message-processing, and security docs.
  Fix CTE typo in retry-mechanism.md.

No breaking changes to public API, Store interface, or CLI flags.

* chore: add CODEOWNERS assigning @formancehq/backend as default reviewers

* fix: remove dead code flagged by ineffassign linter

* fix: address CodeRabbit review feedback

- Return error (nack) when InsertOneAttempt fails for non-success attempts,
  preventing silent loss of retry records
- Rewrite TestProcessMessages_SynchronousProcessing to be deterministic:
  HTTP handler blocks on a channel, test proves handler is blocked until
  the HTTP response completes
- Add TestProcessMessages_NackOnInsertFailureForRetry for the new nack path
- Expand secret logging test patterns to cover %v and more variable names
- Rename TestConfigStringerDoesNotLeakSecret to TestDocument_PercentPlusVLeaksSecrets
- Fix retry-mechanism.md claim query priority documentation: DISTINCT ON
  does per-webhook ordering, not global priority

* fix: propagate errors to span, avoid err shadowing in defer

- Rename loop-scoped err to attemptErr/insertErr so the defer
  closure captures the correct error for span.RecordError()
- Add explicit span.RecordError() on MakeAttempt and InsertOneAttempt
  failures that previously went unrecorded in traces
- Assign err before return on nack path so the defer sees it
- Update Go version in architecture doc to 1.25+ (matches go.mod)

Addresses review comments from paul-nicolas on PR #142.

* fix: handle transport errors in MakeAttempt as retryable attempts

- MakeAttempt now returns a retryable Attempt (instead of a bare error)
  when httpClient.Do fails due to transport/timeout, preventing event
  loss since the attempt record is persisted for the retry loop
- Replace base64-looking test secret with plain dummy to avoid
  triggering secret scanners in CI

Addresses CodeRabbit review comments on PR #142.

2026-04-08 15:16:21 +02:00

2.4 KiB

Raw Permalink Blame History

Security

Webhook Signing

Every webhook delivery is signed using HMAC-SHA256. The signature allows recipients to verify that the webhook originated from the Formance webhooks service and has not been tampered with.

Secret Management

Secrets are 24 random bytes, base64-encoded
A secret is auto-generated when creating a config if none is provided
Secrets can be rotated via PUT /configs/{id}/secret/change
Custom secrets must be exactly 24 bytes (before base64 encoding)

Signature Format

The formance-webhook-signature header contains: v1,<base64-hmac-sha256>

The signed content is: {webhook_id}.{unix_timestamp}.{json_body}

The v1 prefix enables future signature scheme upgrades without breaking existing integrations.

Log Hygiene

The service follows strict rules about what appears in logs:

What is NOT logged

Environment variables — os.Environ() is never dumped to logs, as it typically contains database credentials, API keys, and other secrets
Webhook secrets — Config objects are never logged with %+v or any format that would expose the secret field. Only config.ID and config.Endpoint appear in log messages
Event payloads in traces — Raw event payloads are not stored as OpenTelemetry span attributes, as they may contain sensitive business data

What IS logged (at debug level)

Event types being processed
Config IDs and endpoints matched for delivery
Webhook IDs and delivery status
Retry claim counts and webhook IDs

Authentication

The REST API supports OAuth2 client credentials authentication via the --auth-* flags. When enabled, all config management endpoints require a valid bearer token. The /_healthcheck and /_info endpoints are unauthenticated.

Input Validation

Endpoint URLs are validated (must be parseable, non-empty)
Event types must be non-empty strings
Secrets must be valid base64 encoding exactly 24 bytes when decoded
Request bodies reject unknown JSON fields (DisallowUnknownFields)
Query filters reject unknown filter keys with an error (no silent pass-through)

Database Security

All queries use parameterized statements (via bun ORM) — no SQL injection risk
The atomic claim pattern for retries uses WHERE status = 'to retry' scoping to prevent double-processing
Config deletion verifies existence before deleting (SELECT then DELETE)

2.4 KiB Raw Permalink Blame History