Skip to content

Commit eb01658

Browse files
committed
fix(message-gatekeeper): allow quoting a tshow message from its parent channel
1 parent ad6f8fc commit eb01658

5 files changed

Lines changed: 74 additions & 5 deletions

File tree

docs/client-api.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2384,6 +2384,7 @@ Embedded snapshot of the quoted message at the time of quoting.
23842384
| `messageLink` | string | Optional. |
23852385
| `threadParentId` | string | Optional. Set if the quoted message itself is a thread reply. |
23862386
| `threadParentCreatedAt` | string | Optional. RFC 3339. |
2387+
| `tshow` | boolean | Optional. Mirrors the quoted message's own `tshow` — set when the quoted message is a thread reply also shown in its parent channel room. |
23872388

23882389
When the reader is in a restricted access window and the quoted parent falls outside it, the embedded snapshot is redacted to `{ "msg": "This message is unavailable" }` — all other quote fields are dropped.
23892390

@@ -4650,7 +4651,7 @@ Delivered on `chat.user.{account}.response.{requestId}`. See [Error envelope](#6
46504651
| `not subscribed` | `forbidden` | `not_subscribed` | Sender is not a member of the room. |
46514652
| `posting is restricted to owners and admins in this room` | `forbidden` | `large_room_post_restricted` | Non-owner/admin/bot posting a top-level message in a room above the large-room threshold (thread replies are exempt). |
46524653
| `quoted parent {id} not found` | `not_found` || The quoted message lookup failed (deleted, cross-room, …). |
4653-
| `quoted parent {id} thread context mismatch: …` | `bad_request` || A quoted message must be in the same thread context (main-room or the same thread) as the new message. |
4654+
| `quoted parent {id} thread context mismatch: …` | `bad_request` || A quoted message must be in the same thread context (main-room or the same thread) as the new message — except a `tshow: true` thread reply, which may also be quoted from its parent channel room. |
46544655

46554656
**Delivery guarantee:** every validation/authorization failure — including a `siteID` mismatch and a malformed `msg.send` subject — is replied to the client on the response subject and the JetStream message is acked (not retried). The error reply requires a routable response subject, so it can only be sent when the `{account}` segment is recoverable from the subject and the payload carries a valid hyphenated-UUID `requestId`; if neither is recoverable (a truly malformed subject or missing/invalid `requestId`) no reply is possible and the client falls back to a request timeout. **Only infrastructure failures** (store/publish errors) are nak'd and **redelivered by JetStream** — these produce no immediate reply.
46564657

message-gatekeeper/fetcher_history.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ type quotedParentProjection struct {
5454
DecodedAttachments []cassandra.Attachment `json:"attachments"`
5555
ThreadParentID string `json:"threadParentId"`
5656
ThreadParentCreatedAt *time.Time `json:"threadParentCreatedAt"`
57+
TShow bool `json:"tshow"`
5758
}
5859

5960
// FetchQuotedParent issues a NATS request to history-service's GetMessageByID
@@ -101,5 +102,6 @@ func (f *historyParentFetcher) FetchQuotedParent(
101102
MessageLink: messageLink(f.chatBaseURL, parent.RoomID, messageID),
102103
ThreadParentID: parent.ThreadParentID,
103104
ThreadParentCreatedAt: parent.ThreadParentCreatedAt,
105+
TShow: parent.TShow,
104106
}, nil
105107
}

message-gatekeeper/handler.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ func (h *Handler) resolveQuoteSnapshot(ctx context.Context, account, roomID, sit
433433
"request_id", natsutil.RequestIDFromContext(ctx), "quoted_id", quotedParentMessageID, "error", err)
434434
return ph, true, nil
435435
}
436-
if cerr := checkQuoteThreadContext(snap, quotedParentMessageID, newMessageThreadID); cerr != nil {
436+
if cerr := checkQuoteThreadContext(snap, quotedParentMessageID, newMessageThreadID, roomID); cerr != nil {
437437
return nil, false, cerr
438438
}
439439
return snap, false, nil
@@ -478,11 +478,16 @@ func (h *Handler) placeholderQuoteSnapshot(roomID, messageID, newMessageThreadID
478478
}
479479

480480
// checkQuoteThreadContext enforces the same-conversation rule between the quoted
481-
// parent and the quoting message.
482-
func checkQuoteThreadContext(snap *cassandra.QuotedParentMessage, quotedParentMessageID, newMessageThreadID string) error {
481+
// parent and the quoting message. newMessageRoomID is the quoting message's own
482+
// room, needed for the TShow carve-out below.
483+
func checkQuoteThreadContext(snap *cassandra.QuotedParentMessage, quotedParentMessageID, newMessageThreadID, newMessageRoomID string) error {
484+
// TShow ("also send to channel") carve-out: the parent also lives in its channel
485+
// room, so quoting it from that room (a main-room message) is legitimate.
486+
tshowChannelQuote := snap.TShow && newMessageThreadID == "" && snap.RoomID == newMessageRoomID
483487
if snap.ThreadParentID != newMessageThreadID &&
484488
// Thread-root quote: starter is a main-room msg (ThreadParentID=="") whose ID is the thread parent — allowed.
485-
(snap.ThreadParentID != "" || quotedParentMessageID != newMessageThreadID) {
489+
(snap.ThreadParentID != "" || quotedParentMessageID != newMessageThreadID) &&
490+
!tshowChannelQuote {
486491
return errcode.BadRequest(fmt.Sprintf("quoted parent %s thread context mismatch: parent thread %q, new message thread %q",
487492
quotedParentMessageID, snap.ThreadParentID, newMessageThreadID))
488493
}

message-gatekeeper/handler_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1498,6 +1498,64 @@ func TestHandler_resolveQuoteSnapshot(t *testing.T) {
14981498
assert.Equal(t, errcode.CodeForbidden, ee.Code)
14991499
},
15001500
},
1501+
{
1502+
// chat#432: a TShow ("also send to channel") thread reply is visible in
1503+
// its parent channel room, so quoting it from that room (threadID=="")
1504+
// must be allowed even though the parent's own ThreadParentID is set.
1505+
name: "tshow parent quoted from its parent channel room — allowed",
1506+
quotedID: quotedID,
1507+
threadID: "",
1508+
setupFetcher: func(f *MockParentMessageFetcher) {
1509+
f.EXPECT().FetchQuotedParent(gomock.Any(), account, roomID, siteID, quotedID).
1510+
Return(&cassandra.QuotedParentMessage{MessageID: quotedID, RoomID: roomID, ThreadParentID: "thread-X", TShow: true, Msg: "tshow msg"}, nil)
1511+
},
1512+
wantSnap: true,
1513+
assertSnap: func(t *testing.T, snap *cassandra.QuotedParentMessage) {
1514+
assert.Equal(t, "tshow msg", snap.Msg)
1515+
},
1516+
},
1517+
{
1518+
name: "non-tshow thread reply quoted from the channel — still rejected",
1519+
quotedID: quotedID,
1520+
threadID: "",
1521+
setupFetcher: func(f *MockParentMessageFetcher) {
1522+
f.EXPECT().FetchQuotedParent(gomock.Any(), account, roomID, siteID, quotedID).
1523+
Return(&cassandra.QuotedParentMessage{MessageID: quotedID, RoomID: roomID, ThreadParentID: "thread-X", TShow: false, Msg: "thread-only msg"}, nil)
1524+
},
1525+
wantErr: true,
1526+
assertErr: func(t *testing.T, err error) {
1527+
var ee *errcode.Error
1528+
require.True(t, errors.As(err, &ee))
1529+
assert.Equal(t, errcode.CodeBadRequest, ee.Code)
1530+
},
1531+
},
1532+
{
1533+
// The carve-out is gated on the quoter being in the parent's own room —
1534+
// a TShow parent shown in room A must not be quotable from unrelated room B.
1535+
name: "tshow parent from a different room — still rejected",
1536+
quotedID: quotedID,
1537+
threadID: "",
1538+
setupFetcher: func(f *MockParentMessageFetcher) {
1539+
f.EXPECT().FetchQuotedParent(gomock.Any(), account, roomID, siteID, quotedID).
1540+
Return(&cassandra.QuotedParentMessage{MessageID: quotedID, RoomID: "some-other-room", ThreadParentID: "thread-X", TShow: true, Msg: "tshow msg"}, nil)
1541+
},
1542+
wantErr: true,
1543+
assertErr: func(t *testing.T, err error) {
1544+
var ee *errcode.Error
1545+
require.True(t, errors.As(err, &ee))
1546+
assert.Equal(t, errcode.CodeBadRequest, ee.Code)
1547+
},
1548+
},
1549+
{
1550+
name: "tshow parent quoted from within its own thread — allowed",
1551+
quotedID: quotedID,
1552+
threadID: "thread-X",
1553+
setupFetcher: func(f *MockParentMessageFetcher) {
1554+
f.EXPECT().FetchQuotedParent(gomock.Any(), account, roomID, siteID, quotedID).
1555+
Return(&cassandra.QuotedParentMessage{MessageID: quotedID, RoomID: roomID, ThreadParentID: "thread-X", TShow: true, Msg: "tshow msg"}, nil)
1556+
},
1557+
wantSnap: true,
1558+
},
15011559
}
15021560

15031561
for _, tc := range tests {

pkg/model/cassandra/message.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ type QuotedParentMessage struct {
5656
// embedding the parent's identity so history-service can enforce access-window checks without an extra read.
5757
ThreadParentID string `json:"threadParentId,omitempty" cql:"thread_parent_id"`
5858
ThreadParentCreatedAt *time.Time `json:"threadParentCreatedAt,omitempty" cql:"thread_parent_created_at"`
59+
// TShow mirrors the quoted message's own flag: an "also send to channel"
60+
// thread reply, quotable from its parent channel room (see checkQuoteThreadContext).
61+
TShow bool `json:"tshow,omitempty" cql:"tshow"`
5962
}
6063

6164
// Message represents a message row in the Cassandra message tables

0 commit comments

Comments
 (0)