fix(webhooks): sign outbound webhooks with a dedicated secret, not JWT_SECRET #324

Merged
YousifShkara merged 2 commits from fix/BUNYIP-332-dedicated-webhook-signing-secret into main 2026-07-03 12:54:53 +02:00
Owner

Before this change bunyip-api/src/main.rs constructed the WebhookService with jwt_secret.clone(), meaning every RP that verified a bunyip webhook had to hold bunyip's access-token signing key. That is a far too broad grant: the same secret that verifies an account_deleted payload could also mint valid bunyip access tokens for any user. And any rotation of JWT_SECRET (which happens for its own security reasons) silently broke webhook verification on the receiving side in flight, with no obvious connection between the two symptoms.

Split the two secrets. Read a new BUNYIP_WEBHOOK_SIGNING_SECRET via the shared secret_env helper (which resolves BUNYIP_WEBHOOK_SIGNING_SECRET_FILE first for compose secrets, then the plain env var for dev .env). Same fail-shape as JWT_SECRET: production panics if unset, dev/test falls back to a stable placeholder so just dev still boots. Pass the new value to WebhookService::new; the service itself is unchanged (its signing_secret: String field takes whatever the caller passes).

Rollout: the value MUST match what mokosh-server already holds in its own BUNYIP_WEBHOOK_SECRET on the same host, or every subsequent account-delete webhook fails HMAC verification and the mokosh soft-delete never fires. Wiring the secret into the docker compose stacks (c-01 and nc-01 bunyip-api) is a separate change; this PR is code-only. See BUNYIP-332 for the full rollout checklist including the mokosh-server-side comment fix.

Also verified during triage:

  • WebhookService is used at runtime (constructed at main.rs:378 pre-change / :382 post-change, called from handlers/user.rs::delete_account via fan_out_account_deleted after a real user delete). A prior triage note claiming it was constructed only in tests was mistaken.
  • Bunyip's Application model has no webhook_secret column; the signing secret is service-wide, not per-RP. mokosh-server's comment describing this as "the same per-app registry row in Bunyip's applications table" is misleading and will be corrected in a matching mokosh-server PR.

#BUNYIP-332

Before this change `bunyip-api/src/main.rs` constructed the `WebhookService` with `jwt_secret.clone()`, meaning every RP that verified a bunyip webhook had to hold bunyip's access-token signing key. That is a far too broad grant: the same secret that verifies an `account_deleted` payload could also mint valid bunyip access tokens for any user. And any rotation of JWT_SECRET (which happens for its own security reasons) silently broke webhook verification on the receiving side in flight, with no obvious connection between the two symptoms. Split the two secrets. Read a new `BUNYIP_WEBHOOK_SIGNING_SECRET` via the shared `secret_env` helper (which resolves `BUNYIP_WEBHOOK_SIGNING_SECRET_FILE` first for compose secrets, then the plain env var for dev .env). Same fail-shape as `JWT_SECRET`: production panics if unset, dev/test falls back to a stable placeholder so `just dev` still boots. Pass the new value to `WebhookService::new`; the service itself is unchanged (its `signing_secret: String` field takes whatever the caller passes). Rollout: the value MUST match what mokosh-server already holds in its own `BUNYIP_WEBHOOK_SECRET` on the same host, or every subsequent account-delete webhook fails HMAC verification and the mokosh soft-delete never fires. Wiring the secret into the docker compose stacks (c-01 and nc-01 bunyip-api) is a separate change; this PR is code-only. See BUNYIP-332 for the full rollout checklist including the mokosh-server-side comment fix. Also verified during triage: - `WebhookService` is used at runtime (constructed at main.rs:378 pre-change / :382 post-change, called from `handlers/user.rs::delete_account` via `fan_out_account_deleted` after a real user delete). A prior triage note claiming it was constructed only in tests was mistaken. - Bunyip's `Application` model has no `webhook_secret` column; the signing secret is service-wide, not per-RP. mokosh-server's comment describing this as "the same per-app registry row in Bunyip's `applications` table" is misleading and will be corrected in a matching mokosh-server PR. #BUNYIP-332
fix(webhooks): sign outbound webhooks with a dedicated secret, not JWT_SECRET
Some checks failed
Check / fmt + clippy + build + tests (pull_request) Successful in 41m18s
E2E / Playwright against deployment (pull_request) Failing after 6m48s
393a5780ba
Before this change `bunyip-api/src/main.rs` constructed the `WebhookService` with `jwt_secret.clone()`, meaning every RP that verified a bunyip webhook had to hold bunyip's access-token signing key. That is a far too broad grant: the same secret that verifies an `account_deleted` payload could also mint valid bunyip access tokens for any user. And any rotation of JWT_SECRET (which happens for its own security reasons) silently broke webhook verification on the receiving side in flight, with no obvious connection between the two symptoms.

Split the two secrets. Read a new `BUNYIP_WEBHOOK_SIGNING_SECRET` via the shared `secret_env` helper (which resolves `BUNYIP_WEBHOOK_SIGNING_SECRET_FILE` first for compose secrets, then the plain env var for dev .env). Same fail-shape as `JWT_SECRET`: production panics if unset, dev/test falls back to a stable placeholder so `just dev` still boots. Pass the new value to `WebhookService::new`; the service itself is unchanged (its `signing_secret: String` field takes whatever the caller passes).

Rollout: the value MUST match what mokosh-server already holds in its own `BUNYIP_WEBHOOK_SECRET` on the same host, or every subsequent account-delete webhook fails HMAC verification and the mokosh soft-delete never fires. Wiring the secret into the docker compose stacks (c-01 and nc-01 bunyip-api) is a separate change; this PR is code-only. See BUNYIP-332 for the full rollout checklist including the mokosh-server-side comment fix.

Also verified during triage:
- `WebhookService` is used at runtime (constructed at main.rs:378 pre-change / :382 post-change, called from `handlers/user.rs::delete_account` via `fan_out_account_deleted` after a real user delete). A prior triage note claiming it was constructed only in tests was mistaken.
- Bunyip's `Application` model has no `webhook_secret` column; the signing secret is service-wide, not per-RP. mokosh-server's comment describing this as "the same per-app registry row in Bunyip's `applications` table" is misleading and will be corrected in a matching mokosh-server PR.

#BUNYIP-332
YousifShkara force-pushed fix/BUNYIP-332-dedicated-webhook-signing-secret from 393a5780ba
Some checks failed
Check / fmt + clippy + build + tests (pull_request) Successful in 41m18s
E2E / Playwright against deployment (pull_request) Failing after 6m48s
to 055bcfb307
All checks were successful
E2E / Playwright against deployment (pull_request) Successful in 24s
Check / fmt + clippy + build + tests (pull_request) Successful in 9m38s
2026-07-03 12:16:41 +02:00
Compare
ci: retrigger against rebased tip (post-BUNYIP-331 e2e fix)
All checks were successful
E2E / Playwright against deployment (pull_request) Successful in 36s
Check / fmt + clippy + build + tests (pull_request) Successful in 17m18s
Create release / Create release from merged PR (pull_request) Has been skipped
9f02ec60f6
YousifShkara deleted branch fix/BUNYIP-332-dedicated-webhook-signing-secret 2026-07-03 12:54:53 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/bunyip!324
No description provided.