From 51f2e2fdd53e882a32ad1688c8b663d88cb3292a Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Mon, 26 Jan 2026 12:23:30 -0500 Subject: [PATCH] Manual cherrypick MM-67055: Fix permalink embeds in ws msg (#34893) (#35059) Automatic Merge --- server/channels/api4/post.go | 4 +++ server/channels/api4/post_test.go | 47 +++++++++++++++++++++++++++ server/channels/app/post.go | 2 ++ server/channels/app/post_test.go | 54 +++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+) diff --git a/server/channels/api4/post.go b/server/channels/api4/post.go index 586f813e39..62e719454c 100644 --- a/server/channels/api4/post.go +++ b/server/channels/api4/post.go @@ -982,6 +982,10 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) { return } + // MM-67055: Strip client-supplied metadata.embeds to prevent spoofing. + // This matches createPost behavior. + post.SanitizeInput() + auditRec := c.MakeAuditRecord("updatePost", model.AuditStatusFail) model.AddEventParameterAuditableToAuditRec(auditRec, "post", &post) defer c.LogAuditRecWithLevel(auditRec, app.LevelContent) diff --git a/server/channels/api4/post_test.go b/server/channels/api4/post_test.go index 58d22e1af6..89c33da41b 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -1450,6 +1450,53 @@ func TestUpdatePost(t *testing.T) { assert.NotEqual(t, rpost3.Attachments(), rrupost3.Attachments()) }) + t.Run("should strip spoofed metadata embeds", func(t *testing.T) { + // MM-67055: Verify that client-supplied metadata.embeds are stripped + post := &model.Post{ + ChannelId: channel.Id, + Message: "test message " + model.NewId(), + } + createdPost, _, err := client.CreatePost(context.Background(), post) + require.NoError(t, err) + + // Try to update with spoofed embed + updatePost := &model.Post{ + Id: createdPost.Id, + ChannelId: channel.Id, + Message: "updated message " + model.NewId(), + Metadata: &model.PostMetadata{ + Embeds: []*model.PostEmbed{ + { + Type: model.PostEmbedPermalink, + Data: &model.PreviewPost{ + PostID: "spoofed-post-id", + Post: &model.Post{ + Id: "spoofed-post-id", + UserId: th.BasicUser2.Id, + Message: "This is a spoofed message!", + }, + }, + }, + }, + }, + } + + updatedPost, _, err := client.UpdatePost(context.Background(), createdPost.Id, updatePost) + require.NoError(t, err) + + // Verify spoofed embed was stripped + if updatedPost.Metadata != nil { + assert.Empty(t, updatedPost.Metadata.Embeds, "spoofed embeds should be stripped") + } + + // Double-check by fetching the post + fetchedPost, _, err := client.GetPost(context.Background(), createdPost.Id, "") + require.NoError(t, err) + if fetchedPost.Metadata != nil { + assert.Empty(t, fetchedPost.Metadata.Embeds, "spoofed embeds should not be persisted") + } + }) + t.Run("change message, but post too old", func(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.PostEditTimeLimit = 1 diff --git a/server/channels/app/post.go b/server/channels/app/post.go index 56f5e4daf1..e5c3916547 100644 --- a/server/channels/app/post.go +++ b/server/channels/app/post.go @@ -795,6 +795,8 @@ func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, updateP // Always use incoming metadata when provided, otherwise retain existing if receivedUpdatedPost.Metadata != nil { newPost.Metadata = receivedUpdatedPost.Metadata.Copy() + // MM-67055: Strip embeds - always server-generated. Preserves Priority/Acks for Shared Channels sync. + newPost.Metadata.Embeds = nil } else { // Restore the post metadata that was stripped by the plugin. Set it to // the last known good. diff --git a/server/channels/app/post_test.go b/server/channels/app/post_test.go index e074b72233..7bb3e59fe5 100644 --- a/server/channels/app/post_test.go +++ b/server/channels/app/post_test.go @@ -1791,6 +1791,60 @@ func TestUpdatePost(t *testing.T) { }) } }) + + t.Run("should strip client-supplied embeds", func(t *testing.T) { + // MM-67055: Verify that client-supplied metadata.embeds are stripped. + // This prevents WebSocket message spoofing via permalink embeds. + // + // Note: Priority and Acknowledgements are stored in separate database tables, + // not in post metadata. Shared Channels handles them separately via + // syncRemotePriorityMetadata and syncRemoteAcknowledgementsMetadata after + // calling UpdatePost. See sync_recv.go::upsertSyncPost + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.AddUserToChannel(th.BasicUser, th.BasicChannel) + th.Context.Session().UserId = th.BasicUser.Id + + // Create a basic post + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "original message", + UserId: th.BasicUser.Id, + } + createdPost, _, err := th.App.CreatePost(th.Context, post, th.BasicChannel, model.CreatePostFlags{}) + require.Nil(t, err) + + // Try to update with spoofed embeds (the attack vector) + updatePost := &model.Post{ + Id: createdPost.Id, + ChannelId: th.BasicChannel.Id, + Message: "updated message", + UserId: th.BasicUser.Id, + Metadata: &model.PostMetadata{ + Embeds: []*model.PostEmbed{ + { + Type: model.PostEmbedPermalink, + Data: &model.PreviewPost{ + PostID: "spoofed-post-id", + Post: &model.Post{ + Id: "spoofed-post-id", + UserId: th.BasicUser2.Id, + Message: "Spoofed message from another user!", + }, + }, + }, + }, + }, + } + + updatedPost, _, err := th.App.UpdatePost(th.Context, updatePost, nil) + require.Nil(t, err) + require.NotNil(t, updatedPost.Metadata) + + // Verify embeds were stripped + assert.Empty(t, updatedPost.Metadata.Embeds, "spoofed embeds should be stripped") + }) } func TestSearchPostsForUser(t *testing.T) {