Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/client-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2384,6 +2384,7 @@ Embedded snapshot of the quoted message at the time of quoting.
| `messageLink` | string | Optional. |
| `threadParentId` | string | Optional. Set if the quoted message itself is a thread reply. |
| `threadParentCreatedAt` | string | Optional. RFC 3339. |
| `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. |

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.

Expand Down Expand Up @@ -4650,7 +4651,7 @@ Delivered on `chat.user.{account}.response.{requestId}`. See [Error envelope](#6
| `not subscribed` | `forbidden` | `not_subscribed` | Sender is not a member of the room. |
| `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). |
| `quoted parent {id} not found` | `not_found` | — | The quoted message lookup failed (deleted, cross-room, …). |
| `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. |
| `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. |

**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.

Expand Down
2 changes: 2 additions & 0 deletions message-gatekeeper/fetcher_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ type quotedParentProjection struct {
DecodedAttachments []cassandra.Attachment `json:"attachments"`
ThreadParentID string `json:"threadParentId"`
ThreadParentCreatedAt *time.Time `json:"threadParentCreatedAt"`
TShow bool `json:"tshow"`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check if we store tshow in quotedParentMsg in current behavior ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question — checked on current `main`: no, `tshow` is not stored in the `quoted_parent_message` snapshot. The UDT has no `tshow` field, and nothing writes one into it.

What is persisted is the message's own `tshow` — the `tshow` column on `messages_by_id` (`Message.TShow cql:"tshow"`). So this PR adds `TShow` to the quote projection as transient (`cql:"-"`, never persisted) and populates it per-request from history's by-id reply (fetcher_history.go:105parent.TShow, i.e. the parent row's own persisted flag), not from the stored snapshot. That keeps it schema/UDT-change-free and migration-free — the snapshot stays as-is.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked — no, current (main) behavior does not store tshow on the quoted-parent snapshot, and this PR doesn't persist it either:

  • On main, neither quotedParentProjection nor the cassandra.QuotedParentMessage UDT has a tshow field. The only tshow-related data captured on the parent snapshot is thread_parent_id / thread_parent_created_at (set by message-worker when the quoted message is a TShow reply), which history-service's access-window redaction reads together with the outer message's own m.TShow (history-service/internal/service/utils.go quoteInaccessible).
  • The TShow field this PR adds to QuotedParentMessage is tagged cql:"-" — it is JSON-only, not written to the quoted_parent_message UDT column, so there's no schema/migration change. It's populated transiently at quote-resolution time from the live parent row (FetchQuotedParent reads it via history-service GetMessageByID, whose reply is a full cassandra.Message and already carries tshow) and consumed only in-process by the gatekeeper carve-out in resolveQuoteSnapshot (tshowChannelQuote := snap.TShow && newMessageThreadID == "" && snap.RoomID == newMessageRoomID).

So the flag is authoritative from the live parent at send time; nothing new is stored, and on a later read-back of the persisted snapshot TShow is simply absent (false), which no read path depends on. Happy to drop the field from the UDT struct and thread it through a local var instead if you'd prefer it not live on QuotedParentMessage at all.

}

// FetchQuotedParent issues a NATS request to history-service's GetMessageByID
Expand Down Expand Up @@ -101,5 +102,6 @@ func (f *historyParentFetcher) FetchQuotedParent(
MessageLink: messageLink(f.chatBaseURL, parent.RoomID, messageID),
ThreadParentID: parent.ThreadParentID,
ThreadParentCreatedAt: parent.ThreadParentCreatedAt,
TShow: parent.TShow,
}, nil
}
13 changes: 9 additions & 4 deletions message-gatekeeper/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ func (h *Handler) resolveQuoteSnapshot(ctx context.Context, account, roomID, sit
"request_id", natsutil.RequestIDFromContext(ctx), "quoted_id", quotedParentMessageID, "error", err)
return ph, true, nil
}
if cerr := checkQuoteThreadContext(snap, quotedParentMessageID, newMessageThreadID); cerr != nil {
if cerr := checkQuoteThreadContext(snap, quotedParentMessageID, newMessageThreadID, roomID); cerr != nil {
return nil, false, cerr
}
return snap, false, nil
Expand Down Expand Up @@ -478,11 +478,16 @@ func (h *Handler) placeholderQuoteSnapshot(roomID, messageID, newMessageThreadID
}

// checkQuoteThreadContext enforces the same-conversation rule between the quoted
// parent and the quoting message.
func checkQuoteThreadContext(snap *cassandra.QuotedParentMessage, quotedParentMessageID, newMessageThreadID string) error {
// parent and the quoting message. newMessageRoomID is the quoting message's own
// room, needed for the TShow carve-out below.
func checkQuoteThreadContext(snap *cassandra.QuotedParentMessage, quotedParentMessageID, newMessageThreadID, newMessageRoomID string) error {
// TShow ("also send to channel") carve-out: the parent also lives in its channel
// room, so quoting it from that room (a main-room message) is legitimate.
tshowChannelQuote := snap.TShow && newMessageThreadID == "" && snap.RoomID == newMessageRoomID
if snap.ThreadParentID != newMessageThreadID &&
// Thread-root quote: starter is a main-room msg (ThreadParentID=="") whose ID is the thread parent — allowed.
(snap.ThreadParentID != "" || quotedParentMessageID != newMessageThreadID) {
(snap.ThreadParentID != "" || quotedParentMessageID != newMessageThreadID) &&
!tshowChannelQuote {
return errcode.BadRequest(fmt.Sprintf("quoted parent %s thread context mismatch: parent thread %q, new message thread %q",
quotedParentMessageID, snap.ThreadParentID, newMessageThreadID))
}
Expand Down
58 changes: 58 additions & 0 deletions message-gatekeeper/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1498,6 +1498,64 @@ func TestHandler_resolveQuoteSnapshot(t *testing.T) {
assert.Equal(t, errcode.CodeForbidden, ee.Code)
},
},
{
// chat#432: a TShow ("also send to channel") thread reply is visible in
// its parent channel room, so quoting it from that room (threadID=="")
// must be allowed even though the parent's own ThreadParentID is set.
name: "tshow parent quoted from its parent channel room — allowed",
quotedID: quotedID,
threadID: "",
setupFetcher: func(f *MockParentMessageFetcher) {
f.EXPECT().FetchQuotedParent(gomock.Any(), account, roomID, siteID, quotedID).
Return(&cassandra.QuotedParentMessage{MessageID: quotedID, RoomID: roomID, ThreadParentID: "thread-X", TShow: true, Msg: "tshow msg"}, nil)
},
wantSnap: true,
assertSnap: func(t *testing.T, snap *cassandra.QuotedParentMessage) {
assert.Equal(t, "tshow msg", snap.Msg)
},
},
{
name: "non-tshow thread reply quoted from the channel — still rejected",
quotedID: quotedID,
threadID: "",
setupFetcher: func(f *MockParentMessageFetcher) {
f.EXPECT().FetchQuotedParent(gomock.Any(), account, roomID, siteID, quotedID).
Return(&cassandra.QuotedParentMessage{MessageID: quotedID, RoomID: roomID, ThreadParentID: "thread-X", TShow: false, Msg: "thread-only msg"}, nil)
},
wantErr: true,
assertErr: func(t *testing.T, err error) {
var ee *errcode.Error
require.True(t, errors.As(err, &ee))
assert.Equal(t, errcode.CodeBadRequest, ee.Code)
},
},
{
// The carve-out is gated on the quoter being in the parent's own room —
// a TShow parent shown in room A must not be quotable from unrelated room B.
name: "tshow parent from a different room — still rejected",
quotedID: quotedID,
threadID: "",
setupFetcher: func(f *MockParentMessageFetcher) {
f.EXPECT().FetchQuotedParent(gomock.Any(), account, roomID, siteID, quotedID).
Return(&cassandra.QuotedParentMessage{MessageID: quotedID, RoomID: "some-other-room", ThreadParentID: "thread-X", TShow: true, Msg: "tshow msg"}, nil)
},
wantErr: true,
assertErr: func(t *testing.T, err error) {
var ee *errcode.Error
require.True(t, errors.As(err, &ee))
assert.Equal(t, errcode.CodeBadRequest, ee.Code)
},
},
{
name: "tshow parent quoted from within its own thread — allowed",
quotedID: quotedID,
threadID: "thread-X",
setupFetcher: func(f *MockParentMessageFetcher) {
f.EXPECT().FetchQuotedParent(gomock.Any(), account, roomID, siteID, quotedID).
Return(&cassandra.QuotedParentMessage{MessageID: quotedID, RoomID: roomID, ThreadParentID: "thread-X", TShow: true, Msg: "tshow msg"}, nil)
},
wantSnap: true,
},
}

for _, tc := range tests {
Expand Down
5 changes: 5 additions & 0 deletions pkg/model/cassandra/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ type QuotedParentMessage struct {
// embedding the parent's identity so history-service can enforce access-window checks without an extra read.
ThreadParentID string `json:"threadParentId,omitempty" cql:"thread_parent_id"`
ThreadParentCreatedAt *time.Time `json:"threadParentCreatedAt,omitempty" cql:"thread_parent_created_at"`
// TShow mirrors the quoted message's own flag: an "also send to channel" thread
// reply, quotable from its parent channel room (see checkQuoteThreadContext).
// Transient (cql:"-") — resolved per-request from history's reply, never persisted
// into the quoted_parent_message UDT.
TShow bool `json:"tshow,omitempty" cql:"-"`
}

// Message represents a message row in the Cassandra message tables
Expand Down
14 changes: 14 additions & 0 deletions pkg/model/cassandra/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,20 @@ func TestQuotedParentMessage_JSON_WithThreadParent(t *testing.T) {
assert.Equal(t, parentAt, *got.ThreadParentCreatedAt)
}

func TestQuotedParentMessage_JSON_TShow(t *testing.T) {
q := QuotedParentMessage{MessageID: "m1", RoomID: "r1", Sender: Participant{ID: "u1"}, CreatedAt: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), TShow: true}
b, err := json.Marshal(q)
require.NoError(t, err)
assert.Contains(t, string(b), `"tshow":true`)
assert.True(t, roundTrip(t, q).TShow)

q.TShow = false // omitempty ⇒ absent when false
b, err = json.Marshal(q)
require.NoError(t, err)
assert.NotContains(t, string(b), "tshow")
assert.False(t, roundTrip(t, q).TShow)
}

func TestMessage_JSON(t *testing.T) {
now := time.Date(2026, 1, 15, 12, 0, 0, 0, time.UTC)
edited := now.Add(5 * time.Minute)
Expand Down
Loading