Target credentials leak into the per-webhook event databases via GORM association upsert #206

Closed
opened 2026-08-20 05:47:57 +02:00 by clawbot · 1 comment
Collaborator

Two related exposures of the same secret.

  1. targets.config is stored in plaintext in webhooker.db. For a Slack target the webhookUrl IS the bearer credential; for an http target the URL can embed userinfo. The UI masks these on screen (#113, #115, #118) but the storage is clear.

  2. Worse, they do not stay in that one file. GORM's association upsert on the retry path copies the whole target row, config included, into the PER-WEBHOOK event database, with an empty webhook_id. Confirmed:

select id, webhook_id, name, config from targets;   -- inside events-*.db

returns live destination URLs. So anything that backs up, ships, or hands over an event DB — and event DBs are the thing most likely to be copied around, since they hold the payload history — ships the credentials with it.

Definition of done:

  • the per-webhook event DBs no longer receive target rows at all: disable the association upsert on the delivery/retry write path (GORM Omit(clause.Associations) or a scoped session), so only delivery and event rows are written there
  • a migration or sweep clears any targets rows already written into existing event DBs
  • a test asserts that after a delivery and a retry, events-*.db contains no targets rows
  • credential-bearing target config is encrypted at rest in webhooker.db using the existing session-key material or a dedicated key, OR, if that is judged out of scope for 1.0, that decision is recorded on this issue and the exposure documented in the README backup section

The first three bullets are the blocking part; the encryption question can be split out if it is large.

Two related exposures of the same secret. 1. `targets.config` is stored in plaintext in `webhooker.db`. For a Slack target the `webhookUrl` IS the bearer credential; for an `http` target the URL can embed userinfo. The UI masks these on screen (#113, #115, #118) but the storage is clear. 2. Worse, they do not stay in that one file. GORM's association upsert on the retry path copies the whole target row, `config` included, into the PER-WEBHOOK event database, with an empty `webhook_id`. Confirmed: ``` select id, webhook_id, name, config from targets; -- inside events-*.db ``` returns live destination URLs. So anything that backs up, ships, or hands over an event DB — and event DBs are the thing most likely to be copied around, since they hold the payload history — ships the credentials with it. Definition of done: - the per-webhook event DBs no longer receive target rows at all: disable the association upsert on the delivery/retry write path (GORM `Omit(clause.Associations)` or a scoped session), so only delivery and event rows are written there - a migration or sweep clears any `targets` rows already written into existing event DBs - a test asserts that after a delivery and a retry, `events-*.db` contains no `targets` rows - credential-bearing target config is encrypted at rest in `webhooker.db` using the existing session-key material or a dedicated key, OR, if that is judged out of scope for 1.0, that decision is recorded on this issue and the exposure documented in the README backup section The first three bullets are the blocking part; the encryption question can be split out if it is large.
clawbot added this to the 1.0.0 milestone 2026-08-20 05:47:57 +02:00
Author
Collaborator

Scope for 1.0: the first three bullets only — stop the association upsert writing targets rows into the per-webhook event DBs, sweep the rows already written into existing event DBs, and test that a delivery plus a retry leaves no targets rows there.

Encryption at rest is explicitly OUT of this unit and deferred past 1.0, per the escape hatch in the definition of done. It needs a key-management story (key source, rotation, re-wrap migration) and getting it wrong loses every target config; the file is trusted-operator-only in the meantime. Tracked at #212, and the exposure is documented in the README backup section by #210.

Implementation notes: the sweep must be idempotent and must not fail startup on an event DB that has no targets table at all. Deleting rows is correct here — those rows are junk with an empty webhook_id, not history; delivery history carries no FK to them.

Scope for 1.0: the first three bullets only — stop the association upsert writing `targets` rows into the per-webhook event DBs, sweep the rows already written into existing event DBs, and test that a delivery plus a retry leaves no `targets` rows there. Encryption at rest is explicitly OUT of this unit and deferred past 1.0, per the escape hatch in the definition of done. It needs a key-management story (key source, rotation, re-wrap migration) and getting it wrong loses every target config; the file is trusted-operator-only in the meantime. Tracked at https://git.eeqj.de/sneak/webhooker/issues/212, and the exposure is documented in the README backup section by https://git.eeqj.de/sneak/webhooker/issues/210. Implementation notes: the sweep must be idempotent and must not fail startup on an event DB that has no `targets` table at all. Deleting rows is correct here — those rows are junk with an empty `webhook_id`, not history; delivery history carries no FK to them.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#206