From dfb68583904e43edf5092c80d2913543aee61388 Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Tue, 13 Jan 2026 16:02:07 -0500 Subject: [PATCH 1/2] require siteURL for external-facing slash cmds, use that as host --- server/channels/api4/command_test.go | 135 ++++++++++++++++++++ server/channels/app/command.go | 8 +- server/channels/app/command_autocomplete.go | 7 +- server/i18n/en.json | 4 + 4 files changed, 152 insertions(+), 2 deletions(-) diff --git a/server/channels/api4/command_test.go b/server/channels/api4/command_test.go index 7f332954d91..1bfa52ef17b 100644 --- a/server/channels/api4/command_test.go +++ b/server/channels/api4/command_test.go @@ -6,6 +6,7 @@ package api4 import ( "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "net/url" @@ -1311,14 +1312,17 @@ func TestExecuteInvalidCommand(t *testing.T) { enableCommands := *th.App.Config().ServiceSettings.EnableCommands allowedInternalConnections := *th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections + siteURL := *th.App.Config().ServiceSettings.SiteURL defer func() { th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableCommands = &enableCommands }) th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }) + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.SiteURL = &siteURL }) }() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "127.0.0.0/8" }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = "http://localhost:8065" }) ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { rc := &model.CommandResponse{} @@ -1383,14 +1387,17 @@ func TestExecuteGetCommand(t *testing.T) { enableCommands := *th.App.Config().ServiceSettings.EnableCommands allowedInternalConnections := *th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections + siteURL := *th.App.Config().ServiceSettings.SiteURL defer func() { th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableCommands = &enableCommands }) th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }) + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.SiteURL = &siteURL }) }() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "127.0.0.0/8" }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = "http://localhost:8065" }) token := model.NewId() expectedCommandResponse := &model.CommandResponse{ @@ -1445,14 +1452,17 @@ func TestExecutePostCommand(t *testing.T) { enableCommands := *th.App.Config().ServiceSettings.EnableCommands allowedInternalConnections := *th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections + siteURL := *th.App.Config().ServiceSettings.SiteURL defer func() { th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableCommands = &enableCommands }) th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }) + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.SiteURL = &siteURL }) }() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "127.0.0.0/8" }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = "http://localhost:8065" }) token := model.NewId() expectedCommandResponse := &model.CommandResponse{ @@ -1614,16 +1624,19 @@ func TestExecuteCommandInDirectMessageChannel(t *testing.T) { enableCommands := *th.App.Config().ServiceSettings.EnableCommands allowedInternalConnections := *th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections + siteURL := *th.App.Config().ServiceSettings.SiteURL defer func() { th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableCommands = &enableCommands }) th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }) + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.SiteURL = &siteURL }) }() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1" }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = "http://localhost:8065" }) // create a team that the user isn't a part of team2 := th.CreateTeam(t) @@ -1678,16 +1691,19 @@ func TestExecuteCommandInTeamUserIsNotOn(t *testing.T) { enableCommands := *th.App.Config().ServiceSettings.EnableCommands allowedInternalConnections := *th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections + siteURL := *th.App.Config().ServiceSettings.SiteURL defer func() { th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableCommands = &enableCommands }) th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }) + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.SiteURL = &siteURL }) }() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1" }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = "http://localhost:8065" }) // create a team that the user isn't a part of team2 := th.CreateTeam(t) @@ -1755,16 +1771,19 @@ func TestExecuteCommandReadOnly(t *testing.T) { enableCommands := *th.App.Config().ServiceSettings.EnableCommands allowedInternalConnections := *th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections + siteURL := *th.App.Config().ServiceSettings.SiteURL defer func() { th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableCommands = &enableCommands }) th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }) + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.SiteURL = &siteURL }) }() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1" }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = "http://localhost:8065" }) expectedCommandResponse := &model.CommandResponse{ Text: "test post command response", @@ -1842,3 +1861,119 @@ func TestExecuteCommandReadOnly(t *testing.T) { require.Error(t, err) CheckBadRequestStatus(t, resp) } + +func TestExecuteCommandResponseURLUsesSiteURL(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + client := th.Client + channel := th.BasicChannel + + enableCommands := *th.App.Config().ServiceSettings.EnableCommands + allowedInternalConnections := *th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections + siteURL := *th.App.Config().ServiceSettings.SiteURL + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableCommands = &enableCommands }) + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections + }) + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.SiteURL = &siteURL }) + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "127.0.0.0/8" }) + + // Set a SiteURL that differs from the test client's Host header (localhost). + // This verifies that response_url uses the configured SiteURL, not the Host header. + // Before the fix (MM-67142), response_url would contain "localhost" and fail this check. + expectedSiteURL := "http://mattermost.example.com" + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = expectedSiteURL }) + + var receivedResponseURL string + expectedCommandResponse := &model.CommandResponse{ + Text: "test response_url command response", + ResponseType: model.CommandResponseTypeInChannel, + } + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + err := r.ParseForm() + require.NoError(t, err) + + // Capture the response_url sent by the server + receivedResponseURL = r.FormValue("response_url") + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(expectedCommandResponse); err != nil { + th.TestLogger.Warn("Error while writing response", mlog.Err(err)) + } + })) + defer ts.Close() + + postCmd := &model.Command{ + CreatorId: th.BasicUser.Id, + TeamId: th.BasicTeam.Id, + URL: ts.URL, + Method: model.CommandMethodPost, + Trigger: "testrespurl", + } + + _, appErr := th.App.CreateCommand(postCmd) + require.Nil(t, appErr, "failed to create command") + + _, _, err := client.ExecuteCommand(context.Background(), channel.Id, "/testrespurl") + require.NoError(t, err) + + // Verify response_url starts with the configured SiteURL, not the Host header + require.True(t, strings.HasPrefix(receivedResponseURL, expectedSiteURL), + "response_url should start with configured SiteURL %q, but got %q", expectedSiteURL, receivedResponseURL) +} + +func TestExecuteCustomCommandRequiresSiteURL(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + client := th.Client + channel := th.BasicChannel + + enableCommands := *th.App.Config().ServiceSettings.EnableCommands + allowedInternalConnections := *th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections + siteURL := *th.App.Config().ServiceSettings.SiteURL + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableCommands = &enableCommands }) + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections + }) + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.SiteURL = &siteURL }) + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "127.0.0.0/8" }) + + // Set SiteURL to a valid value first so we can create the command + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = "http://localhost:8065" }) + + // Create a custom command + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprintln(w, `{"text": "response"}`) + })) + defer ts.Close() + + postCmd := &model.Command{ + CreatorId: th.BasicUser.Id, + TeamId: th.BasicTeam.Id, + URL: ts.URL, + Method: model.CommandMethodPost, + Trigger: "customcmd", + } + _, appErr := th.App.CreateCommand(postCmd) + require.Nil(t, appErr, "failed to create command") + + // Now set SiteURL to empty + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = "" }) + + // Try to execute the custom command - should fail because SiteURL is required for custom commands + _, resp, err := client.ExecuteCommand(context.Background(), channel.Id, "/customcmd") + require.Error(t, err) + CheckBadRequestStatus(t, resp) + + // Built-in commands should still work without SiteURL + _, _, err = client.ExecuteCommand(context.Background(), channel.Id, "/echo test") + require.NoError(t, err) +} diff --git a/server/channels/app/command.go b/server/channels/app/command.go index ab0f4e025a1..aff58d51b04 100644 --- a/server/channels/app/command.go +++ b/server/channels/app/command.go @@ -491,11 +491,17 @@ func (a *App) tryExecuteCustomCommand(rctx request.CTX, args *model.CommandArgs, channelMentionMap := a.MentionsToPublicChannels(rctx, message, team.Id) maps.Copy(p, channelMentionMap.ToURLValues()) + // Use configured SiteURL for response_url to prevent SSRF via Host header spoofing (MM-67142) + siteURL := *a.Config().ServiceSettings.SiteURL + if siteURL == "" { + return cmd, nil, model.NewAppError("tryExecuteCustomCommand", "api.command.execute_command.site_url_required.app_error", nil, "", http.StatusBadRequest) + } + hook, appErr := a.CreateCommandWebhook(cmd.Id, args) if appErr != nil { return cmd, nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]any{"Trigger": trigger}, "", http.StatusInternalServerError).Wrap(appErr) } - p.Set("response_url", args.SiteURL+"/hooks/commands/"+hook.Id) + p.Set("response_url", siteURL+"/hooks/commands/"+hook.Id) return a.DoCommandRequest(rctx, cmd, p) } diff --git a/server/channels/app/command_autocomplete.go b/server/channels/app/command_autocomplete.go index 2057e82a68e..f42a13394b5 100644 --- a/server/channels/app/command_autocomplete.go +++ b/server/channels/app/command_autocomplete.go @@ -269,7 +269,12 @@ func (a *App) getDynamicListArgument(rctx request.CTX, commandArgs *model.Comman params.Add("team_id", commandArgs.TeamId) params.Add("root_id", commandArgs.RootId) params.Add("user_id", commandArgs.UserId) - params.Add("site_url", commandArgs.SiteURL) + + // Use configured SiteURL to prevent SSRF via Host header spoofing (MM-67142) + siteURL := *a.Config().ServiceSettings.SiteURL + if siteURL != "" { + params.Add("site_url", siteURL) + } resp, err := a.doPluginRequest(rctx, "GET", dynamicArg.FetchURL, params, nil) diff --git a/server/i18n/en.json b/server/i18n/en.json index 12f70f794dd..9876b3ec1d3 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -691,6 +691,10 @@ "id": "api.command.execute_command.restricted_dm.error", "translation": "Cannot post in a restricted DM" }, + { + "id": "api.command.execute_command.site_url_required.app_error", + "translation": "SiteURL must be configured to use slash commands" + }, { "id": "api.command.execute_command.start.app_error", "translation": "No command trigger found." From 6baf06056bae73062a77435b03ecc94e6b75107f Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Thu, 15 Jan 2026 14:26:48 -0500 Subject: [PATCH 2/2] warn when rewriting the SiteURL of a custom slash command; test --- server/channels/api4/command_test.go | 6 ++++++ server/channels/app/command.go | 6 ++++++ server/i18n/en.json | 4 ++++ 3 files changed, 16 insertions(+) diff --git a/server/channels/api4/command_test.go b/server/channels/api4/command_test.go index 1bfa52ef17b..40366af131f 100644 --- a/server/channels/api4/command_test.go +++ b/server/channels/api4/command_test.go @@ -18,6 +18,7 @@ import ( "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/v8/channels/testlib" ) func TestCreateCommand(t *testing.T) { @@ -1924,6 +1925,10 @@ func TestExecuteCommandResponseURLUsesSiteURL(t *testing.T) { // Verify response_url starts with the configured SiteURL, not the Host header require.True(t, strings.HasPrefix(receivedResponseURL, expectedSiteURL), "response_url should start with configured SiteURL %q, but got %q", expectedSiteURL, receivedResponseURL) + + // Verify warning is logged when Host header differs from SiteURL + require.NoError(t, th.TestLogger.Flush()) + testlib.AssertLog(t, th.LogBuffer, mlog.LvlWarn.Name, "Request hostname differs from configured SiteURL. The configured SiteURL will be used for the slash command response URL.") } func TestExecuteCustomCommandRequiresSiteURL(t *testing.T) { @@ -1972,6 +1977,7 @@ func TestExecuteCustomCommandRequiresSiteURL(t *testing.T) { _, resp, err := client.ExecuteCommand(context.Background(), channel.Id, "/customcmd") require.Error(t, err) CheckBadRequestStatus(t, resp) + CheckErrorID(t, err, "api.command.execute_command.site_url_required.app_error") // Built-in commands should still work without SiteURL _, _, err = client.ExecuteCommand(context.Background(), channel.Id, "/echo test") diff --git a/server/channels/app/command.go b/server/channels/app/command.go index aff58d51b04..e73fc84312a 100644 --- a/server/channels/app/command.go +++ b/server/channels/app/command.go @@ -496,6 +496,12 @@ func (a *App) tryExecuteCustomCommand(rctx request.CTX, args *model.CommandArgs, if siteURL == "" { return cmd, nil, model.NewAppError("tryExecuteCustomCommand", "api.command.execute_command.site_url_required.app_error", nil, "", http.StatusBadRequest) } + if siteURL != args.SiteURL { + rctx.Logger().Warn(i18n.T("api.command.execute_command.provided_site_url_different.app_error"), + mlog.String("request_host", args.SiteURL), + mlog.String("configured_site_url", siteURL), + mlog.String("command", trigger)) + } hook, appErr := a.CreateCommandWebhook(cmd.Id, args) if appErr != nil { diff --git a/server/i18n/en.json b/server/i18n/en.json index 9876b3ec1d3..c17ee24a3e5 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -687,6 +687,10 @@ "id": "api.command.execute_command.not_found.app_error", "translation": "Command with a trigger of '{{.Trigger}}' not found. To send a message beginning with \"/\", try adding an empty space at the beginning of the message." }, + { + "id": "api.command.execute_command.provided_site_url_different.app_error", + "translation": "Request hostname differs from configured SiteURL. The configured SiteURL will be used for the slash command response URL." + }, { "id": "api.command.execute_command.restricted_dm.error", "translation": "Cannot post in a restricted DM"