fix: chat accepts quotes now
This commit is contained in:
@@ -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"`
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,<script>",
|
||||
"no leading slash": "books/bg/2/13",
|
||||
"empty": "",
|
||||
"newline injection": "/books\n/evil",
|
||||
"tab injection": "/books\t/evil",
|
||||
}
|
||||
for name, p := range bad {
|
||||
q := QuoteAttachment{Path: p, Title: "Bg 2.13", Text: "the passage"}
|
||||
if q.valid() {
|
||||
t.Errorf("%s: valid() = true for %q — must be refused", name, p)
|
||||
}
|
||||
}
|
||||
|
||||
// A quote with no passage is not a quote.
|
||||
if (&QuoteAttachment{Path: "/books/bg/2/13", Text: " "}).valid() {
|
||||
t.Error("valid() = true for a blank passage")
|
||||
}
|
||||
// Oversized fields are refused rather than truncated.
|
||||
long := strings.Repeat("x", maxQuoteTextRunes+1)
|
||||
if (&QuoteAttachment{Path: "/books/bg/2/13", Text: long}).valid() {
|
||||
t.Error("valid() = true for an oversized passage")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
-- Rows of the new kind must go before the old constraint can be restored.
|
||||
DELETE FROM message_attachments WHERE kind = 'quote';
|
||||
|
||||
ALTER TABLE message_attachments DROP CONSTRAINT IF EXISTS message_attachments_kind_check;
|
||||
|
||||
ALTER TABLE message_attachments
|
||||
ADD CONSTRAINT message_attachments_kind_check
|
||||
CHECK (kind IN ('image', 'video', 'audio', 'file', 'link', 'entity'));
|
||||
@@ -0,0 +1,18 @@
|
||||
-- A shared passage from the library, sent into a chat.
|
||||
--
|
||||
-- Modelled as an attachment rather than message text so it can be rendered as
|
||||
-- a quote and navigated back to. It reuses the columns the link card already
|
||||
-- has, since the shape is the same thing said differently:
|
||||
--
|
||||
-- url → the ?hl= deep link back to the exact phrase
|
||||
-- title → the source reference ("Bhagavad-gītā 2.13")
|
||||
-- description → the quoted passage itself
|
||||
--
|
||||
-- It is a distinct kind rather than a 'link' because it renders as a quote,
|
||||
-- not a preview card, and because its URL is always our own — the service
|
||||
-- refuses any other origin.
|
||||
ALTER TABLE message_attachments DROP CONSTRAINT IF EXISTS message_attachments_kind_check;
|
||||
|
||||
ALTER TABLE message_attachments
|
||||
ADD CONSTRAINT message_attachments_kind_check
|
||||
CHECK (kind IN ('image', 'video', 'audio', 'file', 'link', 'entity', 'quote'));
|
||||
Reference in New Issue
Block a user