From 7c392eebb73af57567ecab17f9e1b8439dc55327 Mon Sep 17 00:00:00 2001 From: Dylan Jeffers Date: Thu, 10 Sep 2026 12:57:28 -0700 Subject: [PATCH] fix(comms): closing the inbox stops new DMs from earlier blast recipients An artist who sent a blast and later turned off "Allow Messages from Everyone" kept getting new DMs. Two paths let any blast recipient through regardless of the artist's current inbox settings: - chat.create skipped the receiver's inbox settings whenever the receiver had ever blasted the sender, with no regard for settings changed after the blast. - chat_allowed treated a thread holding nothing but the blast seed as an "existing chat", so a recipient who had opened the blast could message the artist at any later time. A blast now grants reply rights only while it is newer than the blaster's most recent inbox settings change. Real conversations are untouched, blasting while closed still lets recipients reply (existing test), and a fresh blast after closing re-opens replies to that blast. Also: an upserted chat_permissions row now refreshes updated_at, and a chat whose latest message is a blast reports recheck_permissions so clients consult the other member's current settings before showing the composer. Co-Authored-By: Claude Fable 5.1 --- api/comms/chat.go | 2 +- api/comms/chat_inbox_closed_test.go | 135 ++++++++++++++++++++++++++++ api/comms/validator.go | 7 ++ api/dbv1/user_chat_row.go | 5 +- ddl/functions/chat_allowed.sql | 13 ++- sql/01_schema.sql | 13 ++- 6 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 api/comms/chat_inbox_closed_test.go diff --git a/api/comms/chat.go b/api/comms/chat.go index fe493782..4533cdf2 100644 --- a/api/comms/chat.go +++ b/api/comms/chat.go @@ -255,7 +255,7 @@ func updatePermissions(db dbv1.DBTX, ctx context.Context, userId int32, permit C insert into chat_permissions (user_id, permits, allowed, updated_at) values ($1, $2, $3, $4) on conflict (user_id, permits) - do update set allowed = $3 where chat_permissions.updated_at < $4 + do update set allowed = $3, updated_at = $4 where chat_permissions.updated_at < $4 `, userId, permit, permitAllowed, messageTimestamp.UTC()) return err } diff --git a/api/comms/chat_inbox_closed_test.go b/api/comms/chat_inbox_closed_test.go new file mode 100644 index 00000000..6c9251e6 --- /dev/null +++ b/api/comms/chat_inbox_closed_test.go @@ -0,0 +1,135 @@ +package comms + +import ( + "context" + "fmt" + "testing" + "time" + + "api.audius.co/database" + "api.audius.co/trashid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// An artist who blasts their followers and later closes their inbox must stop +// receiving new DMs from blast recipients. Threads that hold real messages keep +// working, and a fresh blast sent after closing re-opens replies to it. +func TestChatBlastThenCloseInbox(t *testing.T) { + t0 := time.Now().Add(time.Second * -100).UTC() + t1 := time.Now().Add(time.Second * -90).UTC() + t2 := time.Now().Add(time.Second * -80).UTC() + t3 := time.Now().Add(time.Second * -70).UTC() + t4 := time.Now().Add(time.Second * -60).UTC() + t5 := time.Now().Add(time.Second * -50).UTC() + + pool := database.CreateTestDatabase(t, "test_comms") + defer pool.Close() + ctx := context.Background() + + // user 1 is the artist. 201, 202, 203 follow them before the blast. + database.Seed(pool, database.FixtureMap{ + "users": { + {"user_id": 1, "wallet": "wallet1", "handle": "user1"}, + {"user_id": 201, "wallet": "wallet201", "handle": "user201"}, + {"user_id": 202, "wallet": "wallet202", "handle": "user202"}, + {"user_id": 203, "wallet": "wallet203", "handle": "user203"}, + }, + "follows": { + {"follower_user_id": 201, "followee_user_id": 1, "created_at": t0}, + {"follower_user_id": 202, "followee_user_id": 1, "created_at": t0}, + {"follower_user_id": 203, "followee_user_id": 1, "created_at": t0}, + }, + }) + validator := CreateTestValidator(t, pool, DefaultRateLimitConfig, DefaultTestValidatorConfig) + + chatAllowed := func(from, to int32) bool { + var ok bool + require.NoError(t, pool.QueryRow(ctx, `select chat_allowed($1, $2)`, from, to).Scan(&ok)) + return ok + } + assertMessageAllowed := func(sender int32, chatId string, shouldWork bool) { + rpc := RawRPC{ + Params: []byte(fmt.Sprintf(`{"chat_id": "%s", "message_id": "m-%s-%d", "message": "hi"}`, chatId, chatId, sender)), + } + err := validator.validateChatMessage(ctx, sender, rpc) + if shouldWork { + assert.NoError(t, err) + } else { + assert.ErrorContains(t, err, "Not permitted to send messages to this user") + } + } + upgrade := func(follower int32, ts time.Time) string { + chatId := trashid.ChatID(int(follower), 1) + err := chatCreate(pool, ctx, follower, ts, ChatCreateRPCParams{ + ChatID: chatId, + Invites: []PurpleInvite{ + {UserID: trashid.MustEncodeHashID(int(follower)), InviteCode: "x"}, + {UserID: trashid.MustEncodeHashID(1), InviteCode: "x"}, + }, + }) + require.NoError(t, err) + return chatId + } + + // artist blasts followers with an open inbox + _, err := chatBlast(pool, ctx, 1, t1, ChatBlastRPCParams{ + BlastID: "b_open", + Audience: FollowerAudience, + Message: "hello followers", + }) + require.NoError(t, err) + + // 202 opens the blast into a thread but never replies + chatId_202 := upgrade(202, t2) + + // 203 opens the blast into a thread and replies, so a real conversation exists + chatId_203 := upgrade(203, t2) + require.NoError(t, chatSendMessage(pool, ctx, 203, chatId_203, "reply_203", t2, "203 replying")) + + // while the inbox is open, everyone in the audience can reach the artist + assertChatCreateAllowed(t, ctx, validator, 201, 1, true) + assertMessageAllowed(202, chatId_202, true) + assertMessageAllowed(203, chatId_203, true) + + // artist closes their inbox + require.NoError(t, chatSetPermissions(pool, ctx, 1, ChatPermissionAll, []ChatPermission{ChatPermissionNone}, boolPtr(true), t3)) + + // 201 can no longer start a thread off the old blast + assertChatCreateAllowed(t, ctx, validator, 201, 1, false) + assert.False(t, chatAllowed(201, 1)) + + // 202's thread holds nothing but the blast seed, so it grants no reply rights + assertMessageAllowed(202, chatId_202, false) + assert.False(t, chatAllowed(202, 1)) + + // 203's thread is a real conversation and keeps working + assertMessageAllowed(203, chatId_203, true) + assert.True(t, chatAllowed(203, 1)) + + // a follower's own view of the seed-only thread asks the client to recheck permissions + { + chat, err := getUserChat(pool, ctx, chatMembershipParams{UserID: 202, ChatID: chatId_202}) + require.NoError(t, err) + assert.True(t, chat.LastMessageIsPlaintext) + } + + // artist blasts again while closed: recipients of the new blast may reply to it + _, err = chatBlast(pool, ctx, 1, t4, ChatBlastRPCParams{ + BlastID: "b_closed", + Audience: FollowerAudience, + Message: "hello again", + }) + require.NoError(t, err) + + assertChatCreateAllowed(t, ctx, validator, 201, 1, true) + assert.True(t, chatAllowed(202, 1), "new blast fanned into 202's thread re-opens replies") + + // closing the inbox once more after that blast shuts the door again + require.NoError(t, chatSetPermissions(pool, ctx, 1, ChatPermissionAll, []ChatPermission{ChatPermissionNone}, boolPtr(true), t5)) + assertChatCreateAllowed(t, ctx, validator, 201, 1, false) + assert.False(t, chatAllowed(202, 1)) + assert.True(t, chatAllowed(203, 1)) +} + +func boolPtr(b bool) *bool { return &b } diff --git a/api/comms/validator.go b/api/comms/validator.go index af8d5260..a0331cfa 100644 --- a/api/comms/validator.go +++ b/api/comms/validator.go @@ -576,6 +576,13 @@ func hasNewBlastFromUser(pool *dbv1.DBPools, ctx context.Context, userID int32, where blast.from_user_id = $2 and blast.created_at > (select t from last_permission_change) + -- the blaster's own inbox settings: a blast grants reply rights only + -- while it is newer than the blaster's most recent settings change + and blast.created_at > ( + select coalesce(max(updated_at), to_timestamp(0)) + from chat_permissions + where user_id = $2 + ) and chat_allowed(blast.from_user_id, $1) and not exists ( select 1 from chat_member cm diff --git a/api/dbv1/user_chat_row.go b/api/dbv1/user_chat_row.go index 5afe732b..8962cc5e 100644 --- a/api/dbv1/user_chat_row.go +++ b/api/dbv1/user_chat_row.go @@ -48,7 +48,10 @@ func (row UserChatRow) MarshalJSON() ([]byte, error) { clearedHistoryAt = row.ClearedHistoryAt.Time.UTC().Format(time.RFC3339Nano) } - recheckPermissions := false + // A thread whose latest message is a blast may hold nothing but blast + // seeds; whether the viewer may reply then depends on the other member's + // current inbox settings rather than on the thread existing. + recheckPermissions := row.LastMessageIsPlaintext for _, member := range row.ChatMembers { if member.ClearedHistoryAt.Valid && (row.LastMessageAt == nil || member.ClearedHistoryAt.Time.After(*row.LastMessageAt)) { recheckPermissions = true diff --git a/ddl/functions/chat_allowed.sql b/ddl/functions/chat_allowed.sql index 2164c4f9..40b01108 100644 --- a/ddl/functions/chat_allowed.sql +++ b/ddl/functions/chat_allowed.sql @@ -26,7 +26,10 @@ BEGIN RETURN TRUE; END IF; - -- existing chat takes priority over permissions + -- existing chat takes priority over permissions. + -- A blast message only counts if the blast is newer than to_user's most + -- recent inbox settings change: otherwise an artist who blasts and then + -- closes their inbox would still be reachable by every blast recipient. SELECT COUNT(*) > 0 INTO can_message FROM chat_member member_a JOIN chat_member member_b USING (chat_id) @@ -34,6 +37,14 @@ BEGIN WHERE member_a.user_id = from_user_id AND member_b.user_id = to_user_id AND (member_b.cleared_history_at IS NULL OR chat_message.created_at > member_b.cleared_history_at) + AND ( + chat_message.blast_id IS NULL + OR chat_message.created_at > ( + SELECT COALESCE(MAX(updated_at), to_timestamp(0)) + FROM chat_permissions + WHERE user_id = to_user_id + ) + ) ; IF can_message THEN diff --git a/sql/01_schema.sql b/sql/01_schema.sql index 93751524..9dbaa638 100644 --- a/sql/01_schema.sql +++ b/sql/01_schema.sql @@ -1203,7 +1203,10 @@ BEGIN RETURN TRUE; END IF; - -- existing chat takes priority over permissions + -- existing chat takes priority over permissions. + -- A blast message only counts if the blast is newer than to_user's most + -- recent inbox settings change: otherwise an artist who blasts and then + -- closes their inbox would still be reachable by every blast recipient. SELECT COUNT(*) > 0 INTO can_message FROM chat_member member_a JOIN chat_member member_b USING (chat_id) @@ -1211,6 +1214,14 @@ BEGIN WHERE member_a.user_id = from_user_id AND member_b.user_id = to_user_id AND (member_b.cleared_history_at IS NULL OR chat_message.created_at > member_b.cleared_history_at) + AND ( + chat_message.blast_id IS NULL + OR chat_message.created_at > ( + SELECT COALESCE(MAX(updated_at), to_timestamp(0)) + FROM chat_permissions + WHERE user_id = to_user_id + ) + ) ; IF can_message THEN