Keep only the forum ids of a Discourse webhook event

The Discourse receiver stored each verified event body whole in
core.webhook_events.payload, so every forum signup's email address,
username, name, avatar, profile fields and external id stayed in the
database and its backups, members and non-members alike (#163). Nothing
reads a stored Discourse payload back: the receiver handles the event
from the body it was delivered.

The receiver now parses the body once, after it takes the dedupe key,
and stores only the ids processing reads: {"user":{"id":...}} for
user_created, {"group_user":{"group_id":...,"user_id":...}} for the two
group events. A missing id is stored as 0. A body that does not parse,
or lacks the object its event names, is stored as {} and recorded
failed. Email mode still reads the address from the delivered body.

Two answers change. A signed body that is not JSON, or is empty, failed
the JSONB insert: 500, no row, and redelivery of the same bytes, which
can only fail again. It now gets a failed row and 200, as every
recorded event does. A body Go parses but JSONB rejects (a \u0000
escape, invalid UTF-8) answered 500 on every delivery; its ids are now
stored and it is processed.

docs/models/payments-billing.md says what each receiver stores, and the
comments of scripts/db-reader-role.sql stop calling the column raw
provider payloads; no reader column changes. The OpenSpec change
discourse-events-keep-forum-ids is added with this commit. Ride-along:
the test helper newMapping drops a return value nothing read.

Deliberate breaks, scripts/mutation-check -side new, 29 records, all
CAUGHT:
  the insert stores the raw body (8 rows of both tests)
  keep copies the email into the stored user
  keep drops UserID from the stored group user (both group rows)
  keep swaps GroupID and UserID (both group rows)
  keep takes the user object for every event type (3 rows)
  keep stores a user with id 0 when the user object is nil
  a parse error stores keep of the partly filled view
  the parse-error path answers 500 (the three parse-error rows)
  resolvePerson is called with an empty email
  an event of another type is not dropped
  a group event without group_user reconciles from the user object
  the insert writes the body into provider_environment
  the processed body is the stored one (NUL escape, invalid UTF-8)
  the stored group user omits a zero group_id, and a zero user_id
This commit is contained in:
2026-10-22 19:08:33 -04:00
parent ffe983aad0
commit 5219a45a4b
10 changed files with 729 additions and 33 deletions
+1 -1
View File
@@ -55,7 +55,7 @@ One gloss first, because four rules below depend on it. A **commitment** is a mi
2. **[db]** Stripe linkage is one-to-one per entity: each mapping table holds the console UUID and the Stripe id, both unique, so neither side can be linked twice. For a payment the Stripe id is the charge, one attempt; the payment intent is shared by its attempts and is not unique.
3. **[app]** Stripe ids never live on core tables; every resolution between a console record and a Stripe object goes through a `stripe` mapping row. One codified exception: the attribution column on `core.subscription_changes` stores whatever string triggered the transition — sometimes a Stripe event id, sometimes an internal reason like "eager:checkout-return".
4. **[app]** Subscription fulfillment and invoice projection never trust a webhook payload. A subscription or invoice event yields only an id; the reconciler refetches the authoritative state from the Stripe API at the version the SDK pins, takes a per-object lock so concurrent runs serialize, and converges — running it twice is safe and changes nothing the second time, and events may arrive in any order. A webhook endpoint delivers events in its own API version, which can differ from the SDK's, so reading the object back is also what keeps the projection independent of the endpoint's version. Every amount conversion in the invoice projection is range-checked, so a value too large for its column fails the processing loudly instead of being stored as a wrapped negative number.
5. **[app]** Personal data is scrubbed before any webhook payload is stored: names, emails, and addresses are replaced with a redaction marker; card descriptor fields (brand, last4) are deliberately kept.
5. **[app]** Each webhook receiver decides before the insert what it stores of an event, and neither stores the body as delivered. The Stripe receiver stores the event's object with the values of a fixed list of keys, among them names, email addresses, phone numbers and postal addresses, replaced by a redaction marker at any depth; card descriptor fields (brand, last4) are deliberately kept. The Discourse receiver stores only the forum ids its processing reads: the user's id for `user_created`, and the group's and the user's ids for `user_added_to_group` and `user_removed_from_group`.
6. **[app]** Outbound *catalog* sync goes only through the outbox: the enqueue helper takes the caller's database transaction, so the domain write and the outbox row commit or roll back together. Customer creation is the exception — checkout creates the Stripe customer synchronously when no mapping exists, because the checkout session needs it immediately.
7. **[db]** The scheduled-change vocabulary is closed by CHECK constraints: three change types (plan switch, quantity change, cancellation), three firing triggers (at a date, at period end, at the commitment boundary), four lifecycle states (scheduled, applied, superseded, canceled).
8. **[app]** A subscription has at most one pending scheduled change: every writer supersedes existing scheduled rows before creating a new one. No unique index backs this — the rule lives entirely in the writers.
+84 -25
View File
@@ -115,6 +115,23 @@ func (h *WebhookHandler) serve(w http.ResponseWriter, r *http.Request) {
digest := sha256.Sum256(body)
eventID = hex.EncodeToString(digest[:])
}
// The row's payload holds the forum ids processing reads and nothing
// else of the body (keep): a user_created body carries the user's email
// address, username and signup answers, which the table must not
// hold. A body that does not parse is recorded as {} and fails in
// process, so the row still shows that the event arrived.
var view webhookView
parseErr := json.Unmarshal(body, &view)
stored := storedEvent{}
if parseErr == nil {
stored = keep(eventName, view)
}
payload, err := json.Marshal(stored)
if err != nil {
h.cfg.Logger.Error("discourse webhook payload encode failed", slog.Any("error", err))
http.Error(w, "Internal error", http.StatusInternalServerError)
return
}
// NOT EXISTS rather than ON CONFLICT: the table's unique constraint
// includes received_at (a partitioning requirement), so ON CONFLICT
// would only collapse same-instant duplicates — a redelivery minutes
@@ -129,7 +146,7 @@ func (h *WebhookHandler) serve(w http.ResponseWriter, r *http.Request) {
SELECT 1 FROM core.webhook_events
WHERE provider = $1 AND provider_event_id = $2
)`,
"discourse", eventID, eventName, body)
"discourse", eventID, eventName, payload)
if err != nil {
h.cfg.Logger.Error("discourse webhook event insert failed", slog.Any("error", err))
// Return 500 so Discourse retries — nothing was recorded.
@@ -143,7 +160,7 @@ func (h *WebhookHandler) serve(w http.ResponseWriter, r *http.Request) {
return
}
status, procErr := h.process(r, eventName, body)
status, procErr := h.process(r, eventName, view, parseErr)
if procErr != nil {
h.cfg.Logger.Warn("discourse webhook processing failed",
slog.String("event", eventName), slog.Any("error", procErr))
@@ -168,25 +185,67 @@ func errText(err error) sql.NullString {
return sql.NullString{String: err.Error(), Valid: true}
}
// process resolves the affected person and starts their targeted reconcile.
func (h *WebhookHandler) process(r *http.Request, eventName string, body []byte) (string, error) {
// Shapes verified against live Discourse 3.5.3 (change dir,
// live-verification.md): user events carry a full user serializer under
// "user"; group-membership events carry the bare GroupUser join row under
// "group_user" — forum group id + user id only, no names.
var payload struct {
User *struct {
ID int64 `json:"id"`
Username string `json:"username"`
Email string `json:"email"`
} `json:"user"`
GroupUser *struct {
GroupID int64 `json:"group_id"`
UserID int64 `json:"user_id"`
} `json:"group_user"`
// webhookView is the part of an event body that processing reads. Shapes
// verified against live Discourse 3.5.3 (change dir, live-verification.md):
// user events carry a full user serializer under "user"; group-membership
// events carry the bare GroupUser join row under "group_user" — forum group
// id + user id only, no names.
type webhookView struct {
User *struct {
ID int64 `json:"id"`
Email string `json:"email"`
} `json:"user"`
GroupUser *groupUserView `json:"group_user"`
}
// groupUserView is the part of a group event's group_user object that
// processing reads.
type groupUserView struct {
GroupID int64 `json:"group_id"`
UserID int64 `json:"user_id"`
}
// storedEvent is what core.webhook_events.payload holds for a Discourse
// event: the body's own paths and types, ids only.
type storedEvent struct {
User *storedUser `json:"user,omitempty"`
GroupUser *storedGroupUser `json:"group_user,omitempty"`
}
type storedUser struct {
ID int64 `json:"id"`
}
type storedGroupUser struct {
GroupID int64 `json:"group_id"`
UserID int64 `json:"user_id"`
}
// keep builds the stored value for an event from the ids of the object the
// event names, and from nothing else. It is an allowlist for two reasons.
// A field is stored only if keep copies it by name, so a field Discourse adds
// to a body is not stored. Processing reads the parsed body and never the
// stored row, so what keep omits is still available to process.
func keep(eventName string, view webhookView) storedEvent {
var out storedEvent
switch eventName {
case "user_created":
if view.User != nil {
out.User = &storedUser{ID: view.User.ID}
}
case "user_added_to_group", "user_removed_from_group":
if view.GroupUser != nil {
out.GroupUser = &storedGroupUser{GroupID: view.GroupUser.GroupID, UserID: view.GroupUser.UserID}
}
}
if err := json.Unmarshal(body, &payload); err != nil {
return "failed", fmt.Errorf("parse payload: %w", err)
return out
}
// process resolves the affected person and starts their targeted reconcile.
// It reads the parsed body, view, and the error from parsing it.
func (h *WebhookHandler) process(r *http.Request, eventName string, view webhookView, parseErr error) (string, error) {
if parseErr != nil {
return "failed", fmt.Errorf("parse payload: %w", parseErr)
}
q := dcmod.New(h.cfg.DB)
@@ -194,10 +253,10 @@ func (h *WebhookHandler) process(r *http.Request, eventName string, body []byte)
switch eventName {
case "user_created":
if payload.User == nil {
if view.User == nil {
return "failed", errors.New("user event payload has no user object")
}
personID, err := h.resolvePerson(r, payload.User.ID, payload.User.Email)
personID, err := h.resolvePerson(r, view.User.ID, view.User.Email)
if err != nil {
return "failed", err
}
@@ -212,20 +271,20 @@ func (h *WebhookHandler) process(r *http.Request, eventName string, body []byte)
return "processed", nil
case "user_added_to_group", "user_removed_from_group":
if payload.GroupUser == nil {
if view.GroupUser == nil {
return "failed", errors.New("group event payload has no group_user object")
}
// Only managed groups matter; and only linked persons can be
// reconciled (unlinked drift is the sweep's job). Automatic groups
// (trust_level_*) fire these events on every signup, so the mapping
// filter is load-bearing, not just hygiene.
if _, err := q.GetGroupMappingByDiscourseGroupID(ctx, payload.GroupUser.GroupID); err != nil {
if _, err := q.GetGroupMappingByDiscourseGroupID(ctx, view.GroupUser.GroupID); err != nil {
if errors.Is(err, sql.ErrNoRows) {
return "processed", nil
}
return "failed", fmt.Errorf("mapping lookup: %w", err)
}
link, err := q.GetUserLinkByDiscourseUserID(ctx, payload.GroupUser.UserID)
link, err := q.GetUserLinkByDiscourseUserID(ctx, view.GroupUser.UserID)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return "processed", nil
@@ -16,6 +16,8 @@ import (
"net/http"
"net/http/httptest"
"os"
"reflect"
"strings"
"testing"
"git.coopcloud.tech/wiki-cafe/member-console/internal/identity"
@@ -92,7 +94,7 @@ func (f *webhookFixture) newPerson(email, subj string) string {
return person.PersonID
}
func (f *webhookFixture) newMapping(groupName string, gid int64) dcmod.GroupMapping {
func (f *webhookFixture) newMapping(groupName string, gid int64) {
f.t.Helper()
mapping, err := dcmod.New(f.database).CreateGroupMapping(f.ctx, dcmod.CreateGroupMappingParams{
ResourceKey: "discourse_posting", GroupName: groupName, DiscourseGroupID: gid,
@@ -104,7 +106,6 @@ func (f *webhookFixture) newMapping(groupName string, gid int64) dcmod.GroupMapp
_, _ = f.database.ExecContext(context.Background(),
"DELETE FROM discourse.group_mappings WHERE mapping_id = $1", mapping.MappingID)
})
return mapping
}
func (f *webhookFixture) linkPerson(personID string, uid int64, username string) {
@@ -124,6 +125,19 @@ func (f *webhookFixture) deliver(event string, payload any, sign bool, eventID s
if err != nil {
f.t.Fatal(err)
}
return f.post(event, body, sign, eventID)
}
// deliverRaw signs and posts the bytes it is given, which need not be JSON.
func (f *webhookFixture) deliverRaw(event string, body []byte, eventID string) int {
f.t.Helper()
return f.post(event, body, true, eventID)
}
// post sends the bytes it is given as they are, signed by the test secret
// when sign is set, and returns the response code.
func (f *webhookFixture) post(event string, body []byte, sign bool, eventID string) int {
f.t.Helper()
req := httptest.NewRequest(http.MethodPost, web.WebhookPath, bytes.NewReader(body))
req.Header.Set("X-Discourse-Event", event)
req.Header.Set("X-Discourse-Event-Id", eventID)
@@ -149,6 +163,63 @@ func (f *webhookFixture) eventStatus(eventID string) string {
return status
}
// storedPayload reads the payload column of the event's row.
func (f *webhookFixture) storedPayload(eventID string) string {
f.t.Helper()
var payload string
err := f.database.QueryRowContext(f.ctx,
"SELECT payload::text FROM core.webhook_events WHERE provider = 'discourse' AND provider_event_id = $1",
eventID).Scan(&payload)
if err != nil {
f.t.Fatalf("read stored payload: %v", err)
}
return payload
}
// eventError reads the error_message column of the event's row.
func (f *webhookFixture) eventError(eventID string) string {
f.t.Helper()
var msg sql.NullString
err := f.database.QueryRowContext(f.ctx,
"SELECT error_message FROM core.webhook_events WHERE provider = 'discourse' AND provider_event_id = $1",
eventID).Scan(&msg)
if err != nil {
f.t.Fatalf("read error message: %v", err)
}
return msg.String
}
// rowText reads the whole event row as one text value, so a column added
// later is read too.
func (f *webhookFixture) rowText(eventID string) string {
f.t.Helper()
var text string
err := f.database.QueryRowContext(f.ctx,
`SELECT to_jsonb(w)::text
FROM core.webhook_events w WHERE provider = 'discourse' AND provider_event_id = $1`,
eventID).Scan(&text)
if err != nil {
f.t.Fatalf("read row text: %v", err)
}
return text
}
// requireJSONEqual compares two JSON texts as values, so key order and
// spacing do not matter, and prints both on a mismatch.
func requireJSONEqual(t *testing.T, got, want string) {
t.Helper()
var g, w any
if err := json.Unmarshal([]byte(got), &g); err != nil {
t.Fatalf("stored payload %q is not JSON: %v", got, err)
}
if err := json.Unmarshal([]byte(want), &w); err != nil {
t.Fatalf("expected payload %q is not JSON: %v", want, err)
}
if !reflect.DeepEqual(g, w) {
t.Errorf("stored payload = %s, want %s", got, want)
}
}
// Scenario: invalid or missing signature → rejected, no side effects.
func TestWebhookRejectsBadSignature(t *testing.T) {
f := newWebhookFixture(t, linkage.ModeOIDC)
@@ -283,6 +354,230 @@ func TestWebhookGroupEvents(t *testing.T) {
}
}
// Scenario: the row's payload holds only the forum ids processing reads,
// one case per accepted event type; processing still reads the delivered body.
func TestWebhookStoresOnlyForumIDs(t *testing.T) {
for _, tc := range []struct {
event string
check func(t *testing.T, event string)
}{
{"user_created", checkStoresUserID},
{"user_added_to_group", checkStoresGroupIDs},
{"user_removed_from_group", checkStoresGroupIDs},
} {
t.Run(tc.event, func(t *testing.T) { tc.check(t, tc.event) })
}
}
// checkStoresUserID delivers a user_created body shaped like the forum's user
// serializer in email mode, where processing reads the address.
func checkStoresUserID(t *testing.T, event string) {
f := newWebhookFixture(t, linkage.ModeEmail)
tok := uuid.New().String()[:8]
email := "mail-" + tok + "@example.com"
personID := f.newPerson(email, "subj-"+tok)
personal := map[string]string{
"username": "uname-" + tok,
"name": "Full Name " + tok,
"secondary email": "second-" + tok + "@example.net",
"avatar template": "/user_avatar/forum.example/uname-" + tok + "/{size}/1_2.png",
"signup answer": "answer-" + tok,
"external id": "ext-" + tok,
"email": email,
}
eventID := uuid.New().String()
body := map[string]any{"user": map[string]any{
"id": 4242, "username": personal["username"], "name": personal["name"], "email": email,
"secondary_emails": []string{personal["secondary email"]},
"avatar_template": personal["avatar template"],
"title": "Member", "trust_level": 1, "created_at": "2026-07-20T01:00:00.000Z",
"user_fields": map[string]any{"1": personal["signup answer"]},
"groups": []any{map[string]any{"name": "group-" + tok}},
"external_id": personal["external id"],
}}
if code := f.deliver(event, body, true, eventID); code != http.StatusOK {
t.Fatalf("delivery = %d, want 200", code)
}
requireJSONEqual(t, f.storedPayload(eventID), `{"user":{"id":4242}}`)
if len(f.started) != 1 || f.started[0] != personID {
t.Errorf("starter calls = %v, want [%s]", f.started, personID)
}
text := f.rowText(eventID)
for label, value := range personal {
if strings.Contains(text, value) {
t.Errorf("the row holds the body's %s %q: %s", label, value, text)
}
}
}
// checkStoresGroupIDs delivers a group event on a managed group with a linked
// forum user, carrying the five keys verified against a live forum.
func checkStoresGroupIDs(t *testing.T, event string) {
f := newWebhookFixture(t, linkage.ModeOIDC)
personID := f.newPerson("grp-"+uuid.New().String()[:8]+"@example.com", "subj-"+uuid.New().String()[:8])
groupName := "managed-" + uuid.New().String()[:8]
gid := f.fake.AddGroup(groupName, false)
f.newMapping(groupName, gid)
uid := f.fake.AddUser(discoursetest.FakeUser{Username: "grpuser", Email: "g@example.com", Active: true})
f.linkPerson(personID, uid, "grpuser")
if gid == uid {
t.Fatalf("setup: group id and user id are both %d, so a swap of the two would pass", gid)
}
eventID := uuid.New().String()
body := map[string]any{"group_user": map[string]any{
"group_id": gid, "user_id": uid, "notification_level": 3, "id": 27, "created_at": "2026-07-20T01:00:00.000Z",
}}
if code := f.deliver(event, body, true, eventID); code != http.StatusOK {
t.Fatalf("delivery = %d, want 200", code)
}
requireJSONEqual(t, f.storedPayload(eventID), fmt.Sprintf(`{"group_user":{"group_id":%d,"user_id":%d}}`, gid, uid))
if len(f.started) != 1 || f.started[0] != personID {
t.Errorf("starter calls = %v, want [%s]", f.started, personID)
}
}
// Scenario: a body that cannot be used is recorded as {} and fails, and the
// receiver answers 200 as for any recorded event.
func TestWebhookStoresEmptyPayloadWhenBodyUnusable(t *testing.T) {
for _, tc := range []struct {
name string
event string
body string
errPrefix string // error_message begins with this
errExact string // or equals this
}{
{"not JSON", "user_created", "not json", "parse payload:", ""},
{"empty body", "user_created", "", "parse payload:", ""},
{"wrong-typed field", "user_created", `{"user":{"id":1,"email":7}}`, "parse payload:", ""},
{"user_created without user", "user_created", `{"group_user":{"group_id":5,"user_id":6}}`, "",
"user event payload has no user object"},
{"group event without group_user", "user_added_to_group", `{"user":{"id":7,"email":"{email}"}}`, "",
"group event payload has no group_user object"},
} {
t.Run(tc.name, func(t *testing.T) { checkStoresEmptyPayload(t, tc.event, tc.body, tc.errPrefix, tc.errExact) })
}
}
func checkStoresEmptyPayload(t *testing.T, event, body, errPrefix, errExact string) {
// Email mode with a person whose address "{email}" in a body stands for:
// the group event without group_user names that person in its user object,
// so a group branch that reconciles from the user object would start one.
// No other body names the person.
f := newWebhookFixture(t, linkage.ModeEmail)
email := "mail-" + uuid.New().String()[:8] + "@example.com"
f.newPerson(email, "subj-"+uuid.New().String()[:8])
body = strings.ReplaceAll(body, "{email}", email)
eventID := uuid.New().String()
if code := f.deliverRaw(event, []byte(body), eventID); code != http.StatusOK {
t.Fatalf("delivery = %d, want 200", code)
}
requireJSONEqual(t, f.storedPayload(eventID), `{}`)
if got := f.eventStatus(eventID); got != "failed" {
t.Errorf("event status = %q, want failed", got)
}
msg := f.eventError(eventID)
if errPrefix != "" && !strings.HasPrefix(msg, errPrefix) {
t.Errorf("error_message = %q, want a prefix %q", msg, errPrefix)
}
if errExact != "" && msg != errExact {
t.Errorf("error_message = %q, want %q", msg, errExact)
}
if len(f.started) != 0 {
t.Errorf("starter called %d times for an unusable body", len(f.started))
}
}
// Scenario: an event of another type is acknowledged and writes no row,
// whatever the body carries.
func TestWebhookOtherEventTypeWritesNoRow(t *testing.T) {
f := newWebhookFixture(t, linkage.ModeEmail)
email := "mail-" + uuid.New().String()[:8] + "@example.com"
f.newPerson(email, "subj-"+uuid.New().String()[:8])
eventID := uuid.New().String()
body := map[string]any{"user": map[string]any{"id": 4242, "email": email}}
if code := f.deliver("post_created", body, true, eventID); code != http.StatusOK {
t.Fatalf("delivery = %d, want 200", code)
}
if got := f.eventStatus(eventID); !strings.HasPrefix(got, "absent") {
t.Errorf("event status = %q, want no row", got)
}
if len(f.started) != 0 {
t.Errorf("starter called %d times for an event of another type", len(f.started))
}
}
// Scenario: a body Go parses but JSONB would reject (a \u0000 escape, invalid
// UTF-8) stores the ids and is processed as any other body.
func TestWebhookStoresIDsOfBodyJSONBRejects(t *testing.T) {
for _, tc := range []struct {
name string
username string // spliced into the body as written
}{
{"NUL escape", `a\u0000b`},
{"invalid UTF-8", "a\xffb"},
} {
t.Run(tc.name, func(t *testing.T) {
f := newWebhookFixture(t, linkage.ModeEmail)
email := "mail-" + uuid.New().String()[:8] + "@example.com"
personID := f.newPerson(email, "subj-"+uuid.New().String()[:8])
eventID := uuid.New().String()
body := fmt.Sprintf(`{"user":{"id":5,"username":"%s","email":"%s"}}`, tc.username, email)
if code := f.deliverRaw("user_created", []byte(body), eventID); code != http.StatusOK {
t.Fatalf("delivery = %d, want 200", code)
}
requireJSONEqual(t, f.storedPayload(eventID), `{"user":{"id":5}}`)
if got := f.eventStatus(eventID); got != "processed" {
t.Errorf("event status = %q, want processed", got)
}
if len(f.started) != 1 || f.started[0] != personID {
t.Errorf("starter calls = %v, want [%s]", f.started, personID)
}
})
}
}
// Scenario: a group_user object without group_id or user_id is stored with the
// missing id as 0, as processing reads it.
func TestWebhookStoresIDZeroForGroupUserWithoutAnID(t *testing.T) {
for _, tc := range []struct {
name string
body string
want string
}{
{"no group_id", `{"group_user":{"user_id":6}}`, `{"group_user":{"group_id":0,"user_id":6}}`},
{"no user_id", `{"group_user":{"group_id":5}}`, `{"group_user":{"group_id":5,"user_id":0}}`},
} {
t.Run(tc.name, func(t *testing.T) {
f := newWebhookFixture(t, linkage.ModeOIDC)
eventID := uuid.New().String()
if code := f.deliverRaw("user_added_to_group", []byte(tc.body), eventID); code != http.StatusOK {
t.Fatalf("delivery = %d, want 200", code)
}
requireJSONEqual(t, f.storedPayload(eventID), tc.want)
if len(f.started) != 0 {
t.Errorf("starter called %d times for an id of 0", len(f.started))
}
})
}
}
// Scenario: a user object without an id is stored with id 0, as processing
// reads it.
func TestWebhookStoresIDZeroForUserWithoutID(t *testing.T) {
f := newWebhookFixture(t, linkage.ModeEmail)
eventID := uuid.New().String()
body := map[string]any{"user": map[string]any{"email": "noid-" + uuid.New().String()[:8] + "@example.com"}}
if code := f.deliver("user_created", body, true, eventID); code != http.StatusOK {
t.Fatalf("delivery = %d, want 200", code)
}
requireJSONEqual(t, f.storedPayload(eventID), `{"user":{"id":0}}`)
}
// Guard against silent fixture leaks: fail loudly if cleanup can't run.
func TestMain(m *testing.M) {
code := m.Run()
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-10-22
@@ -0,0 +1,169 @@
## Context
Paths under `web/`, `linkage/`, `client/` and `workflows/` below are in `internal/integrations/discourse/`. Line numbers are on the commit this change starts from.
### The receiver today
The route mounts only when `discourse-webhook-secret` is set. `serve` (`web/webhook.go:85-162`) reads the body, capped at 256 KiB, and verifies `X-Discourse-Event-Signature` before any side effect. It accepts three event names from `X-Discourse-Event` (`user_created`, `user_added_to_group`, `user_removed_from_group`) and answers every other event 200 without writing a row. Its dedupe key is the `X-Discourse-Event-Id` header, or a SHA-256 of the raw body when the header is absent. It then inserts a row into `core.webhook_events` with `payload` set to the raw body bytes, unchanged, under an `INSERT ... WHERE NOT EXISTS` guard (`:125-132`). A duplicate delivery is answered 200 and not processed again.
`process` (`:172-241`) runs in the request. It unmarshals the body into a view of three fields (`:177-187`):
- `user_created` reads `user.id` and `user.email`. In `oidc` and `discourseconnect` mode, `resolvePerson` fetches the forum's admin record by `user.id` and matches its external id to a person; in `email` mode it matches `user.email` against a verified primary email. `user.username` is declared in the view and never read.
- The group events read `group_user.group_id`, which must name a managed group, and `group_user.user_id`, which must name a linked forum user.
A resolved person's targeted reconcile starts with the person's id as its only input; the workflow never sees the body. A body whose `user` object has no `id` parses as id 0. `serve` then sets the row's `status` to `processed` or `failed`, with `error_message`, and answers 200.
### What a body carries
Verified against a live Discourse 3.5.3 when the integration was built: a `user_created` body is `{"user": {...}}`, the full user serializer of 56 keys, among them `id`, `username` and `email`. A group event body is `{"group_user": {"group_id", "user_id", "notification_level", "id", "created_at"}}`, with no names. Automatic groups fire the group events on every signup.
Recalled from Discourse's source and not verified against a forum: the user serializer, seen with admin scope, likely also carries `name`, `avatar_template` (whose path embeds the username), `secondary_emails`, `title`, `trust_level`, `user_fields` (the answers to the forum's signup fields, which an operator defines and which can hold anything), group names, profile fields, and `external_id` when the user has a DiscourseConnect record. This change does not depend on that list, because the receiver keeps an allowlist (D1).
Every forum signup produces a `user_created` row. An account that matches no person is processed with nothing to reconcile, and its body is kept like any other.
### Nothing reads a stored Discourse payload
Every reader of `core.webhook_events` was checked. The Stripe workflows read `payload` by row id, for rows the Stripe receiver wrote; the Stripe ordering check, invoice sweep and boot sweep filter on `provider = 'stripe'`; the operator pages and the overview read counts, types and errors, never the payload; the index on `payload->>'id'` covers only rows whose status is `completed`, which a Discourse row never reaches; the billing "data as of" stamp reads `processed_at`. No Discourse code, page, test or script reads the column. Discourse rows are never retried.
### The rules the stored body breaks
- `docs/database-management.md` ("Keeping the list current") counts as personal data a name, an email address, an identity provider's subject or username, and a raw provider payload. A stored Discourse body is all of these.
- `docs/models/payments-billing.md`, invariant 5: "Personal data is scrubbed before any webhook payload is stored". It was written for Stripe and says "any".
- `design/data-model.md` forbids personal data in every JSONB column (the JSONB Governance Policy), and its `webhook_events` table describes `payload` as "Full raw webhook payload. Must not contain PII per JSONB governance policy." The two sentences of that cell contradict each other for a provider whose bodies carry personal data.
## Goals / Non-Goals
**Goals:**
- The Discourse receiver stores no personal data from an event body.
- Processing of every accepted event that parses is unchanged; the three departures for bodies that do not are listed in D3.
- The model card and the upstream feedback say what is true after the change.
- The confirmation error in the linkage path names the person by id.
**Non-Goals:**
- Rewriting rows stored before the change (D4).
- A write path shared with the Stripe receiver, or a general mechanism for marking fields as personal (D5).
- Retention of old webhook events: a separate design question.
- Other failure messages and log lines that carry personal data (D9).
## Decisions
### D1. The receiver stores an allowlist of the ids processing reads
The stored payload holds the ids that processing reads, and nothing else: `user.id` for `user_created`, and `group_user.group_id` and `group_user.user_id` for the two group events. The receiver builds the value from the typed view, field by field, so a key the forum adds to a body is never stored.
The email address is not kept even though `email` mode reads it. The address is read in the request and consumed there. A retry or replay, if one is ever added, would need the address again in `email` mode, and would have to get it from the forum by `user.id`: `AdminUserRecord` (`client/client.go`) declares an `Email` field, but the receiver fetches the admin record only in `oidc` and `discourseconnect` mode, and Discourse's admin serializer may include the address only when emails are requested, so this change does not claim the fetch is available.
Alternatives considered:
- **Store nothing (`{}`).** Dedupe needs only the event id, and the row keeps type, status, error and times. It loses the link from a failed event to the forum account it concerns, which the forum ids give at no cost in personal data.
- **Reuse the Stripe receiver's denylist** (`ScrubPII`, `internal/integrations/stripe/web/webhook.go`). Its keys are Stripe's vocabulary. On a Discourse body it would redact `email` and `name` and keep `username`, `secondary_emails`, `avatar_template`, `user_fields` and `external_id`. A denylist also fails open: a key Discourse adds in a later version is stored until someone notices.
### D2. The stored value keeps the body's paths and types
The projection keeps Discourse's own nesting and key names, `{"user":{"id":<id>}}` and `{"group_user":{"group_id":<group id>,"user_id":<user id>}}`, with each id a JSON number as Discourse sends it. A query that reads a stored row uses the same path as one that reads a delivered body, for example `payload->'user'->>'id'` to join `discourse.user_links.discourse_user_id`.
The values are those the typed view parsed. A `user` object without an `id` parses as id 0 and is stored as `{"user":{"id":0}}`; processing treats it as today (D3 table).
### D3. The receiver parses once, before the insert
`serve` keeps its order up to the dedupe key: read, verify the signature, check the event name, take the key. It then parses the body into the typed view, builds the projection, and inserts the row with the projection as `payload`. `process` takes the parsed view instead of the body bytes, with the parse error, if any, so it reports the same statuses and error messages as today, except in the three cases listed after the table. Parsing writes nothing, so the rule that a request with a bad signature is rejected without side effects still holds.
The dedupe key does not change. When the event id header is absent, `provider_event_id` is still the SHA-256 of the raw body, a digest from which no field of the body can be read back.
| Delivery | Stored `payload` | `status` | `error_message` | Answer |
|---|---|---|---|---|
| The body cannot be read | no row | n/a | n/a | 400, as today |
| Missing or invalid signature, including a body longer than 256 KiB, whose cut copy does not match its signature | no row | n/a | n/a | 403, as today |
| Event of another type | no row | n/a | n/a | 200, as today |
| Body that is not valid JSON, or is empty | `{}` | `failed` | `parse payload: ...` | 200; **changed**, today the insert fails (see below) |
| Valid JSON with a field of the wrong type (for example `{"user":{"id":1,"email":7}}`) | `{}`: the parse error wins over the partly filled view | `failed` | `parse payload: json: cannot unmarshal ...` | 200 |
| Body Go parses but JSONB rejects (a `\u0000` escape, invalid UTF-8) | the projection | as the event's own processing decides | as today for the event | 200; **changed**, today the insert fails on every delivery and the answer is 500 |
| `user_created` with a `user` object that has no `id` | `{"user":{"id":0}}` | as today | as today | 200 |
| Group event whose `group_user` lacks `group_id` or `user_id` | the missing id stored as 0 | as today | as today | 200 |
| `user_created` with no `user` object | `{}` | `failed` | `user event payload has no user object`, as today | 200 |
| Group event with no `group_user` object | `{}` | `failed` | `group event payload has no group_user object`, as today | 200 |
| `user_created` with a `user` object | `{"user":{"id":<id>}}` | as today | as today | 200 |
| Group event with a `group_user` object | `{"group_user":{"group_id":<g>,"user_id":<u>}}` | as today | as today | 200 |
| Duplicate of a recorded event | the first delivery's row, untouched | unchanged | unchanged | 200, as today |
| The insert fails | no row | n/a | n/a | 500, as today, so Discourse redelivers |
| Encoding the projection fails | no row | n/a | n/a | 500; unreachable while the fields are integers |
| Two simultaneous deliveries of one event | both can insert, as today (the `NOT EXISTS` guard is not atomic) | each processed | each | 200; unchanged, listed for completeness |
| No event-id header, and an add and a remove of one membership | the second has the first's body hash, so it is dropped as a duplicate | n/a | n/a | 200; unchanged, listed for completeness |
| The status update fails | the projection, with status `received` | `received` | none | 200, as today; the failure is logged |
A JSON syntax error names at most one character. A type error names a field and a type, and Go repeats the number when the value is a number that is not an int64 (`cannot unmarshal number 1.5 into Go struct field ... of type int64`); the repeated number is a forum id or a number in a field processing does not read, not a string such as an email address, so `parse payload: ...` carries no personal data into `error_message`, which `console_reader` can read. The projection's fields are integers, so encoding it cannot fail; if the encoder ever returns an error, the receiver treats it as a failed insert: it records nothing and answers 500.
**Where the answer differs from today.** (1) A signed body that is not JSON, or is empty, fails today's JSONB insert, so the receiver answers 500, writes no row, and Discourse redelivers the same bytes. After the change it records `{}` with the status `failed` and answers 200. The receiver's rule is to answer 200 once an event is recorded, because a 5xx is for a failure that recorded nothing and that a redelivery can fix; the same bytes delivered again can only fail to parse again, so a 500 would only make Discourse retry, and with the raw body no longer stored there is nothing the insert can fail on. (2) Naming the view type changes the wording of a type error on the body's top level: the message says `webhookView` where it described an anonymous struct. (3) `Username` leaves the view, so a body whose `user.username` is not a string is processed, where it failed before. The same holds for a body Go accepts and JSONB rejects (a `\u0000` escape or invalid UTF-8): today it answers 500 on every delivery, and after the change it is processed. A `user` object with a field of the wrong type fills the view partly before `json.Unmarshal` returns the error, while a syntax error leaves the view empty; the receiver stores `{}` on any parse error and does not keep the ids of the partly filled view.
### D4. No migration rewrites rows stored before the change
No known deployment has stored a Discourse event, and the project adds no one-off cleanup code for a single deployment before launch. A deployment that ran the Discourse webhook before this change keeps those bodies until its operator rewrites them; one `UPDATE` of the `provider = 'discourse'` rows to the D2 shape does it, and backups taken earlier keep the bodies.
### D5. No shared write path and no general sensitivity mechanism
The Discourse receiver leaves the console with #174, so a write path shared with the Stripe receiver would serve one receiver after that. The general rule, personal fields marked in the integration contract's definitions so that a receiver can tell what it may store, is proposed on #171.
### D6. The reader role and its column list do not change
`console_reader` already withholds `core.webhook_events.payload` through the `\.payload$` pattern in `scripts/db-reader-role.sql`. The forum ids the column now holds for Discourse rows are readable by the project's policy, but the same pattern withholds the Stripe receiver's stored objects and the commands in `core.outbox`, so it stays; only the two comments that call the column "raw provider payloads" are reworded. `internal/migrate/testdata/console-reader-columns.txt` keeps `withhold core.webhook_events.payload`, `TestConsoleReaderColumns` passes without `-update`, and `docs/database-management.md` keeps its definition of personal data, which still counts a raw provider payload.
### D7. The model card's invariant 5
Invariant 5 of `docs/models/payments-billing.md` reads, after the change:
> 5. **[app]** Each webhook receiver decides before the insert what it stores of an event, and neither stores the body as delivered. The Stripe receiver stores the event's object with the values of a fixed list of keys, among them names, email addresses, phone numbers and postal addresses, replaced by a redaction marker at any depth; card descriptor fields (brand, last4) are deliberately kept. The Discourse receiver stores only the forum ids its processing reads: the user's id for `user_created`, and the group's and the user's ids for `user_added_to_group` and `user_removed_from_group`.
### D8. The design-feedback entry
The entry goes at the top of `status/design-feedback.md`, under the file's opening paragraph, in the file's format. Its text:
> ### Webhook events: the payload column is described as the full raw body and as free of personal data
> Labels: `design-feedback`, `integration`, `upstream-membcons-db`
>
> Filed <the landing date> when `discourse-events-keep-forum-ids` landed. `design/` is not edited here; the point is worded to be copied upstream.
>
> `design/data-model.md` describes `webhook_events.payload` as "Full raw webhook payload. Must not contain PII per JSONB governance policy." The two sentences contradict each other for any provider whose event bodies carry personal data: a Discourse `user_created` body carries the forum user's email address and username, and a Stripe customer object carries a name and an address. The table's opening paragraph adds that the table serves "queryable webhook history for debugging and replay", which also reads as the full body. Neither receiver in the console stores the body as delivered. The Stripe receiver stores the event's object with a fixed list of personal keys redacted, and the Discourse receiver stores only the forum ids its processing reads. Upstream fix: describe `payload` as what the receiver keeps of an event, which holds no personal data except what the Stripe receiver keeps on purpose (card brand, last four digits and expiry, which the project's `docs/database-management.md` withholds from its reader role as personal), and say that a receiver that needs more of an event later fetches it from the provider by the ids it kept.
### D9. The ride-along: the confirmation error names the person by id
`confirmCreated` (`linkage/linkage.go:248-265`) runs after the forum reports an account created. When the lookup that follows finds nothing, which is what the forum's enumeration protection causes when the email address is already taken, it returns:
forum reported user created for %q but post-create lookup found nothing (duplicate email swallowed by enumeration protection?)
with the person's primary email address in the `%q`. `createForumUser` returns that error unwrapped (`:239`), and so does `EnsureLink` (`:138-141`). From there it reaches two places:
- the sweep's `linkEntitledPersons` logs it at Warn with the person's id (`workflows/activities.go:128-132`);
- the targeted reconcile's `reconcilePerson` wraps it as `ensure link: ...` (`:357-358`) and returns it from `ReconcilePersonActivity`, so Temporal records the string as the activity's failure in the workflow history.
The error names the person by id instead (`for person %s`, with `ident.PersonID`), and keeps the rest of its text.
Two neighbours stay as they are:
- The error at `linkage/linkage.go:241` names the username seed, which `usernameBase` takes from the email address's local part. It cannot run: the loop's last attempt (`attempt` 4) never continues, so it always returns at `:233` or `:239`.
- Other copies of personal data in error text stay outside this change. Three leaks are near the one fixed here:
- `client/client.go:193-195`: `do` returns the `net/http` error as it is, and that error repeats the request URL, including the percent-encoded `?email=` of `FindUserByEmail`, on failed connections, timeouts and cancellations. It reaches `resolve`, `confirmCreated` in `email` mode, the sweep's log line and Temporal.
- `workflows/activities.go:386` and `:396`: the add and remove errors name the person's forum username, which the forum's own error text can also repeat.
- `linkage/linkage.go:241`: names the username seed derived from the email's local part; unreachable.
#162 covers the copies in Temporal's failure messages (its "Done when" says workflow IDs, search attributes and failure messages carry no personal data). It does not cover the console's own logs, the sweep's Warn line and the worker's activity-error log. The log copies are #250.
## Tests
Tests follow `docs/testing.md`. The receiver tests use the existing fixture in `web/webhook_test.go`, which opens the database through `dbtest.Open`, commits its rows because the handler reads through the pool, and deletes them in cleanup. The linkage test uses the rolled-back fixture in `linkage/linkage_test.go` and the fake forum in `discoursetest`, whose create handler already answers a duplicate email with success and creates nothing.
- **The stored payload, per accepted event type.** Each case delivers a signed body built like a real one and reads the row's `payload` back, comparing it with the expected object as JSON values, so key order and spacing do not matter; a failure prints both. The `user_created` body carries `id`, `username`, `name`, `email`, `secondary_emails`, `avatar_template`, `title`, `trust_level`, `created_at`, `user_fields` with one answer, `groups` with one name and `external_id`. It is delivered in `email` mode, where processing reads the address, and the case asserts that the person's reconcile started, so the test shows that processing still reads the delivered body. The group bodies carry the five verified `group_user` keys, on a managed group with a linked person, and each case asserts the reconcile start.
- **The bodies that store `{}`.** A body that is not JSON, an empty body, a `user_created` body with a wrong-typed field (`{"user":{"id":1,"email":7}}`), a `user_created` body without `user`, and a group body without `group_user`: each answers 200, stores `{}`, ends `failed` with an `error_message` that begins `parse payload:` (the first three) or names the missing object (the last two), and starts no reconcile. The wrong-typed row proves that the ids of a partly filled view are not stored.
- **A user without an id.** A `user_created` body whose `user` object has no `id` stores `{"user":{"id":0}}`.
- **Nothing personal anywhere in the row.** The personal-fields case reads every text column of the row and asserts that none contains any of the body's personal values.
- **The confirmation error.** In `oidc` mode with auto-create on, the fake holds a forum user with the person's email address and no external id, so the lookup finds nobody, the create is swallowed, and the confirmation finds nothing. `EnsureLink` returns an error that contains the person's id and does not contain the email address.
- **Deliberate breaks.** Each new test is proved with `scripts/mutation-check` on the new side; the breaks are listed in `tasks.md` and in each commit message.
## Risks / Trade-offs
- **A stored Discourse row no longer shows the body for debugging.** The forum ids lead to the forum account, whose record the admin API returns. Nothing in the console read the body, and `console_reader` could not see it.
- **The unverified field list.** The test body includes fields recalled from Discourse's source. The receiver does not depend on that list, because it keeps an allowlist; a field the list misses is not stored either.
- **Rows at a deployment the project does not know about.** They keep their bodies until an operator rewrites them (D4). The rows have no reader, so leaving them changes no behaviour.
</content>
</invoke>
@@ -0,0 +1,47 @@
## Why
The Discourse webhook receiver writes each verified event body unchanged into `core.webhook_events.payload` (`internal/integrations/discourse/web/webhook.go`, the insert in `serve`). A `user_created` body is Discourse's full user serializer: it carries the forum user's email address and username, and, depending on how the forum is set up, the person's name, secondary email addresses, the answers to the forum's signup fields and, recalled from Discourse's source and not verified against a forum, an identity provider's subject. Every forum signup produces one of these rows, whether or not the account belongs to a member, and nothing deletes them. Nothing reads them back either: the receiver parses the body in memory and processes the event inside the request, and no code, page, workflow, test or script selects the payload of a Discourse row. The column keeps personal data that no reader needs, against the project's own rule for this column (`docs/models/payments-billing.md`, invariant 5) and its definition of personal data (`docs/database-management.md`, "Keeping the list current").
## What Changes
- **The receiver stores the forum ids and nothing else.** It parses the body before the insert and stores only the ids that processing reads, under the paths the body uses: `{"user":{"id":<id>}}` for `user_created`, and `{"group_user":{"group_id":<group id>,"user_id":<user id>}}` for `user_added_to_group` and `user_removed_from_group`. A body that does not parse, or that lacks the object its event names, is stored as `{}`. A user with no id is stored as id 0, as processing reads it. The stored value is built from an allowlist, so a field that Discourse adds to an event's body in a later version is not stored.
- **Processing does not change for a body that parses and was stored.** It reads the parsed body in the request, as it does today: the same lookups and reconcile starts. In `email` linkage mode the receiver still matches the body's email address against a verified primary email; the address is read in memory and never written. The unused `Username` field of the parsed view goes.
- **Three departures from today's statuses and messages, all deliberate.** (1) A signed body that is not JSON, or is empty, fails today's insert into the JSONB column: the receiver answers 500, writes no row, and Discourse redelivers the same bytes. After the change the row is recorded with `{}` and the status `failed`, and the receiver answers 200. The receiver already answers 200 for every event it has recorded, because a redelivery cannot change the outcome of a failure that recording did not cause, and the same bytes can only fail to parse again. (2) Naming the view type changes the wording of a type error on the body's top level (`of type web.webhookView` in place of the anonymous struct's description). (3) Dropping `Username` from the view means a body whose `user.username` is not a string is now processed, where it failed before. A body that Go accepts but JSONB rejects (a `\u0000` escape, invalid UTF-8) answered 500 on every delivery and is processed after the change.
- **The model card states what each receiver keeps.** Invariant 5 of `docs/models/payments-billing.md` says that personal data is scrubbed before any webhook payload is stored, which the Discourse receiver makes false today. It is reworded to say what the Stripe receiver and the Discourse receiver each store.
- **The data model's contradiction is recorded for upstream.** `design/data-model.md` describes `webhook_events.payload` as the "Full raw webhook payload" and, in the same cell, says it "Must not contain PII". An entry in `status/design-feedback.md` asks upstream to describe the column as what the receiver keeps of an event.
## Ride-along fix
Listed here because it is a quick fix outside the receiver. It lands as its own commit with its own test.
- **A failed confirmation of a created forum account names the person by id** (relates to #162). When the forum reports an account created and the lookup that follows finds nothing, `confirmCreated` (`internal/integrations/discourse/linkage/linkage.go:262`) returns an error that contains the person's primary email address. The sweep logs that error (`linkEntitledPersons`), and the targeted reconcile returns it to Temporal, which records it as the activity's failure in the workflow history (`reconcilePerson`). The error names the person by id instead.
- **The billing views' "data as of" stamp reads Stripe's events only** (ride-along). The operator billing views are projected from Stripe's webhook events, but their recency stamp takes the newest processed event of any provider, so a processed forum event moves it forward while Stripe's stream may have stopped. The stamp's query gains the provider condition (`internal/server/operator_billing.go`, `latestProcessedWebhookEventAt`).
## Capabilities
### New Capabilities
None.
### Modified Capabilities
- `discourse-integration-infrastructure`: added "The webhook receiver stores only the forum ids that processing reads". The existing "Verified webhook ingestion" requirement says nothing about what is stored and stays as it is.
- `operator-billing-views`: modified "Billing views disclose projection recency", so the stamp follows Stripe's events only.
## Impact
- **Schema:** none, and no migration. No row stored before the change is rewritten (design D4).
- **Reader role:** `console_reader` keeps withholding `core.webhook_events.payload`. The pattern that withholds it (`\.payload$` in `scripts/db-reader-role.sql`) also covers the Stripe receiver's stored objects and the commands in `core.outbox`, which still need it. The two comments in `scripts/db-reader-role.sql` that call the column "raw provider payloads" are reworded (comments only). `internal/migrate/testdata/console-reader-columns.txt` does not change, and no deployment needs to re-run the reader script for this change.
- **Code:** `internal/integrations/discourse/web/webhook.go` and `internal/integrations/discourse/linkage/linkage.go`.
- **Tests:** `internal/integrations/discourse/web/webhook_test.go` (the stored payload for each accepted event type, and for a body that does not parse) and `internal/integrations/discourse/linkage/linkage_test.go` (the confirmation error). No golden file changes.
- **Docs:** `docs/models/payments-billing.md` and `status/design-feedback.md`. `docs/database-management.md` does not change: its definition of personal data still counts a raw provider payload, and this change stops storing one.
- **UI:** none. No page reads the payload of a Discourse row.
## Out of scope
- **One write path for `core.webhook_events`, and a general rule for what a receiver may store.** The Discourse receiver leaves the console with #174, so a write path shared with the Stripe receiver would serve one receiver after that. The general rule, personal fields marked in the integration contract's definitions, is proposed on #171.
- **Retention of old webhook events.** It is a separate design question; the Stripe receiver's old rows have readers, and a cutoff needs its own design.
- **Other copies of personal data in failure messages and logs.** The targeted reconcile's errors name the person's forum username, and a failed connection during the client's lookup by email repeats the request URL, which carries the address. #162 covers the copies in Temporal's failure messages; the copies in the console's own logs (the sweep's Warn line, the worker's activity-error log) are #250 (design D9).
</content>
</invoke>
@@ -0,0 +1,50 @@
## ADDED Requirements
### Requirement: The webhook receiver stores only the forum ids that processing reads
The Discourse webhook receiver SHALL store in `core.webhook_events.payload` only the forum ids that processing of the event reads, under the paths the event's body uses, with the JSON types it uses, and nothing else from the body: `{"user":{"id":<user id>}}` for a `user_created` event, and `{"group_user":{"group_id":<group id>,"user_id":<user id>}}` for a `user_added_to_group` or `user_removed_from_group` event. A user object without an `id` SHALL be stored with the id 0, as processing reads it. A body that does not parse (including an empty body, and a body with a field of the wrong type, whatever part of it was read before the error), or that lacks the object its event names, SHALL be stored as `{}`, with the status `failed`, and the receiver SHALL answer 200. The receiver SHALL build the stored value from these ids alone, so that a field the forum adds to an event's body is not stored. Processing SHALL read the event from the delivered body within the request, never from the stored row, and an event of a type the receiver does not accept SHALL write no row.
#### Scenario: A user_created event stores the user's id
- **WHEN** a signed `user_created` event arrives whose `user` object has the id 4242
- **THEN** the event's row SHALL be recorded with the payload `{"user":{"id":4242}}`
#### Scenario: A user_added_to_group event stores the group's and the user's ids
- **WHEN** a signed `user_added_to_group` event arrives whose `group_user` object carries `group_id` 41, `user_id` 4242, `notification_level`, `id` and `created_at`
- **THEN** the event's row SHALL be recorded with the payload `{"group_user":{"group_id":41,"user_id":4242}}`
#### Scenario: A user_removed_from_group event stores the group's and the user's ids
- **WHEN** a signed `user_removed_from_group` event arrives whose `group_user` object carries `group_id` 41, `user_id` 4242, `notification_level`, `id` and `created_at`
- **THEN** the event's row SHALL be recorded with the payload `{"group_user":{"group_id":41,"user_id":4242}}`
#### Scenario: Personal fields in a body are not stored
- **WHEN** a signed `user_created` event arrives whose `user` object carries, besides its id, the user's email address, username, name, secondary email addresses, avatar template, answers to the forum's signup fields (`user_fields`) and an external id
- **THEN** the stored payload SHALL equal `{"user":{"id":<user id>}}`
- **AND** none of those values SHALL appear in any text column of the event's row
#### Scenario: A user without an id is stored with id 0
- **WHEN** a signed `user_created` event arrives whose `user` object has no `id`
- **THEN** the event's row SHALL be recorded with the payload `{"user":{"id":0}}`
#### Scenario: Email-mode processing reads the address from the delivered body
- **WHEN** the linkage mode is `email` and a signed `user_created` event arrives whose `user.email` is a person's verified primary email address
- **THEN** the receiver SHALL start that person's targeted reconcile
- **AND** the stored payload SHALL equal `{"user":{"id":<user id>}}`
#### Scenario: A body that cannot be processed is stored empty
- **WHEN** a signed event of an accepted type arrives whose body is not valid JSON or is empty, or whose `user` object has a field of the wrong type (`{"user":{"id":1,"email":7}}`), or a signed `user_created` event arrives with no `user` object, or a signed group event arrives with no `group_user` object
- **THEN** the receiver SHALL answer 200 and the event's row SHALL be recorded with the payload `{}` and the status `failed`
- **AND** no reconcile SHALL start
#### Scenario: An event of another type writes no row
- **WHEN** a signed event arrives whose `X-Discourse-Event` is not `user_created`, `user_added_to_group` or `user_removed_from_group`
- **THEN** the receiver SHALL answer 200 and SHALL write no row to `core.webhook_events`
</content>
</invoke>
@@ -0,0 +1,20 @@
## MODIFIED Requirements
### Requirement: Billing views disclose projection recency
Because operator billing data is projected from processed Stripe webhook events, each operator billing view SHALL display when the most recent Stripe webhook event was processed (e.g. "Last updated at …"). Events from other providers, which share the webhook event store, SHALL NOT move the stamp, because they project nothing into the billing views. When no Stripe webhook event has ever been processed, the views SHALL say nothing about the stream (maintainer, 2026-08-30: the zero-events banner conflated the provider stream with the records below it, reading as "no billing exists" above seeded or locally-created rows; the stream's health is the Stripe integration settings surface's concern).
#### Scenario: Recency stamp renders with data
- **WHEN** an operator views a billing table and Stripe webhook events have been processed
- **THEN** the view shows the most recent processed Stripe event time
#### Scenario: A quiet stream says nothing
- **WHEN** an operator views a billing table on a deployment where no Stripe webhook event has ever been processed
- **THEN** no recency line and no zero-events statement renders
#### Scenario: A forum event does not move the stamp
- **WHEN** a Discourse webhook event is processed after the most recent Stripe event
- **THEN** the billing views show the Stripe event's processing time, not the forum event's
@@ -0,0 +1,53 @@
Tests follow `docs/testing.md`: database tests open the database through `dbtest`, the receiver tests keep the committed-rows fixture of `web/webhook_test.go` with its cleanup, and tests meet the same complexity limit as production code, so a table runs its rows through a named function. A test that asserts a stored payload compares it with the expected object as JSON values, never as text, and prints both on failure.
Sections 1 to 3 are one commit each, and their items are that commit's parts; section 4 is not a commit. Every commit passes `make test` with the test stack up, which runs `make lint` first, including `TestConsoleReaderColumns` and `filler-copy`. No golden file, screen or existing test result changes in any section; one that changes is a stop. Paths under `web/`, `linkage/`, `client/` and `workflows/` are in `internal/integrations/discourse/`.
**Breaks.** Each section's list is written to a file outside the repository and run with `go run ./scripts/mutation-check -list <file> -side new` on the section's commit, after its green control run; the tests are new, so there is no old side. Every record must be CAUGHT. An `old` text below is the code as the section's tasks write it; when the committed code words it differently, the record is corrected to the committed text before the run, never dropped. The commit message lists every break with its result.
## 1. The receiver stores only the forum ids
- [x] 1.1 In `web/webhook.go`, split the parsed view out of `process` into a named type, `webhookView`, with `User` (`ID`, `Email`) and `GroupUser` (`GroupID`, `UserID`); the unused `Username` field goes. A `user` object without an `id` keeps id 0. Add `storedEvent`, with `User` (`ID`) and `GroupUser` (`GroupID`, `UserID`) under the JSON keys `user`, `id`, `group_user`, `group_id` and `user_id`, each object omitted when nil, and a function `keep(eventName string, view webhookView) storedEvent` that copies only the ids of the object the event names (design D1, D2): `user` for `user_created`, `group_user` for the two group events, and nothing when that object is nil. Its doc comment says that it is an allowlist, that processing reads the parsed body and never the stored row, and that a field Discourse adds to a body is therefore not stored.
- [x] 1.2 `serve` (design D3): after it takes the dedupe key, it unmarshals the body into a `webhookView`, keeping the error. On a parse error the stored value is `storedEvent{}`; otherwise it is `keep(eventName, view)`. It marshals that value and passes it to the insert in place of `body`; a marshal error is logged and answered 500, recording nothing, as a failed insert is. `process` takes the view and the parse error in place of the body: a parse error returns `failed` with `parse payload: %w` as today (the same text the code writes now; only the type name inside a top-level type error changes, design D3), and the rest of `process` and `resolvePerson` is unchanged. On a parse error `serve` records `{}` and answers 200 like any other recorded event, where today the insert of a body that is not JSON fails and answers 500 (design D3); the comment above the insert says what the row's `payload` holds, and why. `serve` stays under gocognit's limit, with no new exception in `.golangci.yml`.
- [x] 1.3 Tests in `web/webhook_test.go`. Split `deliver` so that a new `deliverRaw(event string, body []byte, eventID string) int` signs and posts the bytes it is given, and `deliver` marshals and calls it. Add `storedPayload(eventID string)`, which reads the row's `payload`, and a comparison of two JSON values that reports both on a mismatch. `TestWebhookStoresOnlyForumIDs`, one row per accepted event type through a named function, each delivering a signed body built like a real one (design.md "Tests"):
- `user_created` in `email` mode: a person whose verified primary email the body's `user.email` names; a body whose `user` object carries `id`, `username`, `name`, `email`, `secondary_emails`, `avatar_template`, `title`, `trust_level`, `created_at`, `user_fields` with one answer, `groups` with one name, and `external_id`. The stored payload equals `{"user":{"id":<id>}}`, and the person's reconcile started.
- `user_added_to_group` and `user_removed_from_group`: a managed group and a linked forum user; a `group_user` object with the five verified keys (`group_id`, `user_id`, `notification_level`, `id`, `created_at`). The stored payload equals `{"group_user":{"group_id":<group id>,"user_id":<user id>}}`, and the linked person's reconcile started. The case fails its setup when the fixture's group id equals its user id, so a swap of the two cannot pass.
Personal values: the personal-fields case reads every text column of the row (the stored `payload` cast to text, `provider_event_id`, `event_type`, `status`, `error_message`) and asserts that none contains the body's email address, username, name, secondary email, avatar template, signup-field answer or external id.
`TestWebhookStoresEmptyPayloadWhenBodyUnusable`, one row per body through a named function: a `user_created` body that is not JSON, an empty body, a `user_created` body `{"user":{"id":1,"email":7}}` (valid JSON, wrong type), a `user_created` body with no `user` object that carries a `group_user` object instead, and a `user_added_to_group` body with no `group_user` object that carries a `user` object with an email address instead, so that neither of the last two equals `{}`. Each answers 200, stores `{}`, ends `failed`, and starts no reconcile. The first three assert that `error_message` begins `parse payload:`; the last two assert the text of the missing-object error. The wrong-typed row's person exists in `email` mode with id 1's email unreachable, so that only the stored value tells a partly filled view from an empty one. `TestWebhookStoresIDZeroForUserWithoutID` delivers `{"user":{"email":"..."}}` in `email` mode and asserts the stored payload `{"user":{"id":0}}`. The existing tests pass unchanged.
- [x] 1.4 Breaks, run against the new tests:
- the insert stores the raw body (its `payload` argument becomes `body`): caught by every row of both tests, the not-JSON row because the insert of invalid JSON fails and records no row;
- `keep` copies the email into the stored user: caught by the `user_created` row;
- `keep` drops `UserID` from the stored group user: caught by both group rows;
- `keep` swaps `GroupID` and `UserID`: caught by both group rows;
- `keep` takes the user object for every event type (its `user_created` case also lists the two group events): caught by both group rows;
- `keep` stores a user with id 0 when the `user` object is nil: caught by the row with no `user` object;
- on a parse error the stored value is `keep(eventName, view)` of the partly filled view in place of `storedEvent{}`: caught by the wrong-typed row, whose stored payload then holds the user's id (the wrong-typed body is built so that `id` parses before `email` fails);
- the parse-error path answers 500 in place of 200: caught by the status assertion of every unusable row;
- `resolvePerson` is called with `""` in place of the view's email: caught by the `user_created` row's reconcile assertion.
- [x] 1.5 `docs/models/payments-billing.md`: invariant 5 takes the wording of design D7.
- [x] 1.6 `scripts/db-reader-role.sql`: the header comment (near lines 9-10, "raw provider payloads") and the comment on the `\.payload$` pattern (near line 85, "raw provider events and outbound commands") are reworded to what the column holds now: provider event data that a receiver keeps and the commands in `core.outbox`. Comments only: the pattern, the list and the role are unchanged.
- [x] 1.7 No golden moves. `internal/migrate/testdata/console-reader-columns.txt` does not change, and `TestConsoleReaderColumns` passes without `-update` (design D6).
## 2. Ride-alongs: the confirmation error names the person by id, and the billing stamp reads Stripe's events
- [ ] 2.1 In `confirmCreated` (`linkage/linkage.go:262`), the error names the person by id: `for person %s` with `ident.PersonID` in place of `for %q` with `ident.PrimaryEmail`; the rest of the text stays. The error at `:241` stays as it is (design D9).
- [ ] 2.2 Test in `linkage/linkage_test.go`, `TestAutoCreateUnconfirmedErrorNamesPersonByID`: `oidc` mode with auto-create on; the fake holds a forum user whose email is the person's and who has no external id, and whose username differs from the seed `usernameBase` derives, so the lookup by subject finds nobody, the fake swallows the create as a duplicate email, and the confirmation finds nothing. `EnsureLink` returns an error that contains the person's id and does not contain the person's email address; no link is recorded.
- [ ] 2.3 Breaks:
- the error carries the email again (`ident.PrimaryEmail` added beside the id): caught by the no-email assertion;
- the error drops the id (`ident.PersonID` replaced by a fixed word): caught by the person-id assertion.
- [ ] 2.4 The billing views' recency stamp (`latestProcessedWebhookEventAt`, `internal/server/operator_billing.go`) reads only Stripe's events: its query gains `provider = 'stripe'`, and its doc comment says that the views are projected from Stripe's events and that other providers' events, which share the table, project nothing into them (spec `operator-billing-views`, "Billing views disclose projection recency").
- [ ] 2.5 Test in `internal/server`, beside the existing tests of the billing views' recency line: with a processed Stripe event and a later processed Discourse event, the stamp is the Stripe event's processing time; with only a processed Discourse event, no recency line renders. Break, run with section 2's list: the `provider = 'stripe'` condition removed, caught by both cases.
- [ ] 2.6 No golden moves.
## 3. The data model's contradiction, recorded for upstream
- [ ] 3.1 `status/design-feedback.md`: add the entry of design D8 at the top, directly under the file's opening paragraph, with the landing date in its "Filed" line. `design/` is not edited.
## 4. Landing
- [ ] 4.1 With the test stack up, `make test` passes (it runs `make lint` first), and `openspec validate discourse-events-keep-forum-ids --strict` passes.
- [ ] 4.2 Post a comment on #163 under CONTRIBUTING.md's rules for forge text, and close it. The comment states what the receiver now stores for each accepted event type and for a body it cannot use, that processing reads the delivered body as before, with the commit of section 1; that a signed body that is not JSON now gets a recorded `failed` row and a 200 answer, where it got a 500 answer and no row; that rows stored before that commit are not rewritten, because no known deployment has stored a Discourse event; that retention of old webhook events is a separate question; and that a rule shared by every receiver is proposed on #171, since the Discourse receiver leaves the console with #174.
- [ ] 4.3 Post a comment on #162 under the same rules: the error that the linkage path returned when a created forum account could not be confirmed carried the person's email address into the logs and into the targeted reconcile's activity failure in Temporal, and now names the person by id, with the commit of section 2. It then lists the copies that remain in Temporal's failure messages: the forum username in the targeted reconcile's add and remove errors, which the forum's own error text can also repeat, and the `net/http` error that repeats the request URL with the percent-encoded `?email=` when a lookup by email fails to connect, times out or is cancelled, which reaches the confirmation error in `email` mode, the lookup that resolves a forum user and the sweep's log line as well as Temporal. The copies in the console's own logs are #250.
</content>
</invoke>
+6 -5
View File
@@ -6,10 +6,11 @@
-- messages and public DNS names in the console's schemas, and never reads
-- the columns in the `personal` list below: names, email addresses,
-- identity-provider subjects, IP addresses, card details, operators' notes
-- on grants, changes and name rules, workspace descriptions, and raw
-- provider payloads. It is not anonymous: an id still links one person's
-- payments and times, and a site domain that spells a member's name leads
-- to that member's organization.
-- on grants, changes and name rules, workspace descriptions, and the
-- provider event data that receivers keep and the commands in core.outbox.
-- It is not anonymous: an id still links one person's payments and times,
-- and a site domain that spells a member's name leads to that member's
-- organization.
--
-- Run it as the role that owns the console's schemas, which also needs
-- CREATEROLE, and again after every deploy that applies a migration:
@@ -82,7 +83,7 @@ DECLARE
-- ids. internal/migrate/testdata/console-reader-columns.txt records what
-- the list withholds, and TestConsoleReaderColumns checks it.
personal text[] := ARRAY[
'\.payload$', -- raw provider events and outbound commands
'\.payload$', -- provider event data a receiver keeps, and outbound commands
'^core\.persons\.(display_name|primary_email)$',
'^core\.users\.(oidc_subject|last_login_ip)$',
'^core\.organizations\.name$', -- a personal organization carries its person's name