Skip to content

fix(alert): tenant-scope alert keys to prevent cross-tenant collisions#967

Merged
alexluong merged 1 commit into
hookdeck:mainfrom
mvanhorn:fix/955-alert-keys-tenant-scope
Jun 22, 2026
Merged

fix(alert): tenant-scope alert keys to prevent cross-tenant collisions#967
alexluong merged 1 commit into
hookdeck:mainfrom
mvanhorn:fix/955-alert-keys-tenant-scope

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

Summary

Tenant-scope the consecutive-failure alert keys in the Redis alert store. The key format changes from {deployment}alert:{destinationID}:cf (and the cfeval companion) to {deployment}alert:{tenantID}:{destinationID}:cf / :cfeval.

getFailuresKey and getEvaluatedKey now take the tenantID that IncrementConsecutiveFailureCount, MarkAttemptEvaluated, and ResetConsecutiveFailureCount already receive. No other behavior changes: the deployment prefix and the 24h TTL are preserved.

Why this matters

Destination IDs are only unique within a tenant, and the create API accepts a caller-supplied id. Two tenants that both use a destination id like prod previously collided on a single shared key, corrupting consecutive-failure counts and auto-disable behavior across tenants (#955).

No migration code is needed: existing cf/cfeval keys carry a 24h TTL and are reset on success, so any orphaned old-format keys age out on their own. This is the lazy in-place migration alexbouchardd greenlit on the issue.

Testing

  • gofmt -l, go vet ./internal/alert/..., and go build ./... all pass.
  • Added TestAlertStoreTenantIsolation: two tenants sharing the same destination id increment independently, evaluated markers stay tenant-scoped, and resetting one tenant leaves the other's counter intact (the regression Alert Redis keys are not tenant-scoped — cross-tenant collision when destination IDs match #955 reports).
  • The existing deployment-isolation, idempotency, reset, and mark-evaluated tests continue to cover the rest of the surface. The alert store tests require a Redis instance (testutil.CreateTestRedisClient); they run in CI.

Fixes #955

@alexluong alexluong left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM to me!

@alexluong alexluong merged commit 87a2d36 into hookdeck:main Jun 22, 2026
2 checks passed
@mvanhorn

Copy link
Copy Markdown
Contributor Author

Appreciate this fix, @alexluong. Tenant-scoping the alert keys to prevent cross-tenant collisions is an important correctness fix.

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.

Alert Redis keys are not tenant-scoped — cross-tenant collision when destination IDs match

2 participants