From f89410374104d2faa4ac31504cd15ff0896d69f9 Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Wed, 28 Jan 2026 11:53:28 -0500 Subject: [PATCH] manual cherrypick: [MM-67074] Integration Action memory use fix (#34896) (#35089) Automatic Merge --- server/channels/app/integration_action.go | 3 +- .../channels/app/integration_action_test.go | 113 ++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) diff --git a/server/channels/app/integration_action.go b/server/channels/app/integration_action.go index d7594db324..d466ede39e 100644 --- a/server/channels/app/integration_action.go +++ b/server/channels/app/integration_action.go @@ -252,7 +252,8 @@ func (a *App) DoPostActionWithCookie(c request.CTX, postID, actionId, userID, se defer resp.Body.Close() var response model.PostActionIntegrationResponse - respBytes, err := io.ReadAll(resp.Body) + limitedReader := io.LimitReader(resp.Body, MaxIntegrationResponseSize) + respBytes, err := io.ReadAll(limitedReader) if err != nil { return "", model.NewAppError("DoPostActionWithCookie", "api.post.do_action.action_integration.app_error", nil, "", http.StatusBadRequest).Wrap(err) } diff --git a/server/channels/app/integration_action_test.go b/server/channels/app/integration_action_test.go index c9d543faf5..ab648ab49b 100644 --- a/server/channels/app/integration_action_test.go +++ b/server/channels/app/integration_action_test.go @@ -10,6 +10,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "testing" "time" @@ -173,6 +174,118 @@ func TestPostActionEmptyResponse(t *testing.T) { }) } +// infiniteReader generates unlimited data for testing response size limits +type infiniteReader struct{} + +func (r infiniteReader) Read(p []byte) (n int, err error) { + for i := range p { + p[i] = 'a' + } + return len(p), nil +} + +// MM-67074: TestPostActionResponseSizeLimit verifies that DoPostActionWithCookie +// properly limits response sizes to prevent OOM attacks +func TestPostActionResponseSizeLimit(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + channel := th.BasicChannel + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1" + }) + + t.Run("large valid JSON response is truncated", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Send response larger than MaxIntegrationResponseSize (1MB) + // Response starts as valid JSON but becomes truncated + _, _ = io.Copy(w, io.MultiReader( + strings.NewReader(`{"update":{"message":"`), + infiniteReader{}, + strings.NewReader(`"}}`), + )) + })) + defer server.Close() + + interactivePost := model.Post{ + Message: "Interactive post", + ChannelId: channel.Id, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: th.BasicUser.Id, + Props: model.StringInterface{ + model.PostPropsAttachments: []*model.SlackAttachment{ + { + Text: "hello", + Actions: []*model.PostAction{ + { + Type: model.PostActionTypeButton, + Name: "action", + Integration: &model.PostActionIntegration{ + URL: server.URL, + }, + }, + }, + }, + }, + }, + } + + post, _, err := th.App.CreatePostAsUser(th.Context, &interactivePost, "", true) + require.Nil(t, err) + attachments, ok := post.GetProp(model.PostPropsAttachments).([]*model.SlackAttachment) + require.True(t, ok) + + // Should return error due to truncated JSON, but NOT crash or OOM + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, + attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) + require.NotNil(t, err) + // Truncated JSON causes unmarshal error + assert.Equal(t, "api.post.do_action.action_integration.app_error", err.Id) + }) + + t.Run("large invalid response is truncated", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Send infinite non-JSON data + _, _ = io.Copy(w, infiniteReader{}) + })) + defer server.Close() + + interactivePost := model.Post{ + Message: "Interactive post", + ChannelId: channel.Id, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: th.BasicUser.Id, + Props: model.StringInterface{ + model.PostPropsAttachments: []*model.SlackAttachment{ + { + Text: "hello", + Actions: []*model.PostAction{ + { + Type: model.PostActionTypeButton, + Name: "action", + Integration: &model.PostActionIntegration{ + URL: server.URL, + }, + }, + }, + }, + }, + }, + } + + post, _, err := th.App.CreatePostAsUser(th.Context, &interactivePost, "", true) + require.Nil(t, err) + attachments, ok := post.GetProp(model.PostPropsAttachments).([]*model.SlackAttachment) + require.True(t, ok) + + // Should return error due to invalid JSON, but NOT crash or OOM + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, + attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) + require.NotNil(t, err) + assert.Equal(t, "api.post.do_action.action_integration.app_error", err.Id) + }) +} + func TestPostAction(t *testing.T) { mainHelper.Parallel(t) testCases := []struct {