From 18bac9a22d799fcc5dae832bb419cbc994acda1e Mon Sep 17 00:00:00 2001 From: Krispire Admin Date: Sat, 29 Aug 2026 11:58:20 +0530 Subject: [PATCH] fix: chat accepts quotes now --- internal/handler/chat.go | 20 ++++- internal/model/chat.go | 4 + internal/service/chat_service.go | 93 ++++++++++++++++++++++- internal/service/chat_service_test.go | 47 ++++++++++++ migrations/0051_quote_attachment.down.sql | 8 ++ migrations/0051_quote_attachment.up.sql | 18 +++++ 6 files changed, 186 insertions(+), 4 deletions(-) create mode 100644 migrations/0051_quote_attachment.down.sql create mode 100644 migrations/0051_quote_attachment.up.sql diff --git a/internal/handler/chat.go b/internal/handler/chat.go index 8e0831e..e79516c 100644 --- a/internal/handler/chat.go +++ b/internal/handler/chat.go @@ -36,6 +36,8 @@ func writeChatError(w http.ResponseWriter, err error) { response.Error(w, http.StatusNotFound, "conversation not found") case errors.Is(err, service.ErrAvatarUnset): response.Error(w, http.StatusNotFound, err.Error()) + case errors.Is(err, service.ErrChatBadQuote): + response.Error(w, http.StatusBadRequest, err.Error()) case errors.Is(err, service.ErrChatBadAvatarKey): response.Error(w, http.StatusBadRequest, err.Error()) case errors.Is(err, repository.ErrMessageNotHere): @@ -186,7 +188,10 @@ type sendReq struct { // when they dismissed it. Only the URL — never the card's contents. PreviewURL *string `json:"preview_url"` Attachments []mediaReq `json:"attachments"` - ReplyToID *string `json:"reply_to_id"` + // quote is a passage shared from the library. `path` must be app-relative; + // the service refuses anything that could navigate off-site. + Quote *quoteReq `json:"quote"` + ReplyToID *string `json:"reply_to_id"` // forward_from_id is deliberately NOT accepted yet: forwarding lands with // its own "can the caller see the source?" check in a later phase, and // accepting an unvalidated message reference in the meantime would let a @@ -210,11 +215,18 @@ func (h *ChatHandler) Send(w http.ResponseWriter, r *http.Request) { FileName: a.FileName, }) } + var quote *service.QuoteAttachment + if req.Quote != nil { + quote = &service.QuoteAttachment{ + Path: req.Quote.Path, Title: req.Quote.Title, Text: req.Quote.Text, + } + } msg, err := h.svc.Send(r.Context(), userID, convID, service.SendRequest{ Body: req.Body, ClientMsgID: req.ClientMsgID, ReplyToID: req.ReplyToID, PreviewURL: req.PreviewURL, + Quote: quote, Attachments: media, }) if err != nil { @@ -554,6 +566,12 @@ func (h *ChatHandler) UpdateGroup(w http.ResponseWriter, r *http.Request) { response.NoContent(w) } +type quoteReq struct { + Path string `json:"path"` + Title string `json:"title"` + Text string `json:"text"` +} + type receiptReq struct { Seq int64 `json:"seq"` } diff --git a/internal/model/chat.go b/internal/model/chat.go index f6344a1..75ff9b8 100644 --- a/internal/model/chat.go +++ b/internal/model/chat.go @@ -167,6 +167,10 @@ const ( AttachmentFile = "file" AttachmentLink = "link" AttachmentEntity = "entity" + // AttachmentQuote is a passage shared from the library: url is an + // app-relative deep link back to the phrase, title the source reference, + // description the passage itself. + AttachmentQuote = "quote" ) // Attachment is a media file, link preview or shared entity on a message. diff --git a/internal/service/chat_service.go b/internal/service/chat_service.go index 8386dd4..c002cf9 100644 --- a/internal/service/chat_service.go +++ b/internal/service/chat_service.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "net/url" "path" "strings" "time" @@ -41,6 +42,7 @@ var ( ErrAvatarUnset = errors.New("this group has no photo") ErrChatNotAccepting = errors.New("this person isn't accepting messages from you") ErrChatBadWhoCanMessage = errors.New("unknown messaging restriction") + ErrChatBadQuote = errors.New("that quote is not a valid passage reference") ErrChatBadAccountAge = errors.New("account age must be between 1 and 120 months") ErrChatGenderUnset = errors.New("add your gender in profile settings to use this option") ) @@ -377,6 +379,9 @@ type SendRequest struct { // fetches the title, description and image itself, because a client that // could supply them could make any link claim to be anything. PreviewURL *string + // Quote is a passage shared from the library, rendered as a quote the + // reader can tap to return to the exact phrase. + Quote *QuoteAttachment // Attachments are files already uploaded to the private bucket via // PresignAttachment; their keys are checked against the caller's prefix. Attachments []MediaAttachment @@ -407,10 +412,14 @@ func (s *ChatService) Send(ctx context.Context, userID, conversationID string, r return nil, err } + if req.Quote != nil && !req.Quote.valid() { + return nil, ErrChatBadQuote + } + body := strings.TrimSpace(req.Body) - // A photo with no caption is an ordinary message; only a message with - // neither text nor attachments is empty. - if body == "" && len(req.Attachments) == 0 { + // A photo or a shared passage with no caption is an ordinary message; only + // a message with nothing at all in it is empty. + if body == "" && len(req.Attachments) == 0 && req.Quote == nil { return nil, ErrChatEmptyBody } if len([]rune(body)) > maxBodyRunes { @@ -445,6 +454,7 @@ func (s *ChatService) Send(ctx context.Context, userID, conversationID string, r return s.withAttachments(ctx, msg) } + s.attachQuote(ctx, conversationID, msg, req.Quote) s.attachMedia(ctx, conversationID, userID, msg, req.Attachments) s.attachLinkPreview(ctx, conversationID, msg, entities, req.PreviewURL) @@ -499,6 +509,83 @@ func (s *ChatService) attachMedia(ctx context.Context, conversationID, userID st } } +// QuoteAttachment is a passage shared out of the library. +type QuoteAttachment struct { + // Path is an APP-RELATIVE deep link, e.g. "/books/bg/2/13?hl=…". + // + // Deliberately not a full URL. A quote renders as trusted in-app content, + // so allowing an absolute URL would let a sender dress an off-site link up + // as a scripture citation. A path cannot leave the app, which removes the + // question of which origins to trust. + Path string + // Title is the source reference shown above the passage. + Title string + // Text is the quoted passage itself. + Text string +} + +const ( + maxQuoteTitleRunes = 200 + maxQuoteTextRunes = 2000 + maxQuotePathBytes = 2048 +) + +func (q *QuoteAttachment) valid() bool { + if strings.TrimSpace(q.Text) == "" { + return false + } + if len([]rune(q.Title)) > maxQuoteTitleRunes || len([]rune(q.Text)) > maxQuoteTextRunes { + return false + } + return safeAppPath(q.Path) +} + +// safeAppPath accepts only a path within this app. +// +// "//evil.com" and "/\evil.com" are protocol-relative and would navigate +// off-site, so a leading slash alone is not enough to check. +func safeAppPath(p string) bool { + if p == "" || len(p) > maxQuotePathBytes { + return false + } + if p[0] != '/' { + return false + } + if strings.HasPrefix(p, "//") || strings.HasPrefix(p, "/\\") { + return false + } + if strings.ContainsAny(p, "\\") { + return false + } + for _, r := range p { + if r < 0x20 || r == 0x7f { + return false + } + } + // A parsed path must not carry a scheme or host of its own. + u, err := url.Parse(p) + if err != nil || u.Scheme != "" || u.Host != "" { + return false + } + return true +} + +// attachQuote stores a shared passage. Unlike a link card, the contents come +// from the client — but they are only ever rendered as a quotation, and the +// path has already been constrained to this app. +func (s *ChatService) attachQuote(ctx context.Context, conversationID string, msg *model.Message, q *QuoteAttachment) { + if q == nil { + return + } + path, title, text := q.Path, strings.TrimSpace(q.Title), strings.TrimSpace(q.Text) + if _, err := s.chat.AddAttachment(ctx, repository.AddAttachmentParams{ + MessageID: msg.ID, ConversationID: conversationID, Ord: 0, + Kind: model.AttachmentQuote, URL: &path, Title: &title, Description: &text, + }); err != nil { + s.logger.Warn("chat: attach quote failed", zap.Error(err)) + } +} + // attachLinkPreview unfurls the chosen link and stores the card. // // Two rules make this safe. The URL must be one the parser actually found in diff --git a/internal/service/chat_service_test.go b/internal/service/chat_service_test.go index cf0410d..9263855 100644 --- a/internal/service/chat_service_test.go +++ b/internal/service/chat_service_test.go @@ -7,6 +7,7 @@ import ( "errors" "os" "path/filepath" + "strings" "testing" "time" @@ -1719,3 +1720,49 @@ func TestOpenDMByHandle(t *testing.T) { t.Errorf("unknown handle: err = %v, want ErrUserGone", err) } } + +// A quote renders as trusted in-app content, so its link must be unable to +// leave the app. Anything that could navigate off-site has to be refused. +func TestQuoteAttachment_RejectsOffSitePaths(t *testing.T) { + good := []string{ + "/books/bg/2/13", + "/books/bg/2/13?hl=abc%20def", + "/u/someone/posts#post-1", + "/", + } + for _, p := range good { + q := QuoteAttachment{Path: p, Title: "Bg 2.13", Text: "the passage"} + if !q.valid() { + t.Errorf("valid() = false for in-app path %q", p) + } + } + + bad := map[string]string{ + "absolute http": "http://evil.example/x", + "absolute https": "https://evil.example/x", + "protocol relative": "//evil.example/x", + "backslash relative": "/\\evil.example/x", + "scheme only": "javascript:alert(1)", + "data uri": "data:text/html,