Merge 6baf06056b into 0263262ef4

2026-02-03 20:40:00 -05:00 · 2026-02-04 03:04:17 +02:00 · 2026-02-04 03:04:17 +02:00 · 0331113698
commit 0331113698
parent 0263262ef4 6baf06056b
4 changed files with 168 additions and 2 deletions
--- 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"
@ -17,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) {
@ -1311,14 +1313,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 +1388,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 +1453,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 +1625,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 +1692,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 +1772,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 +1862,124 @@ 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)
+
+	// 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) {
+	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)
+	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")
+	require.NoError(t, err)
+}
--- a/server/channels/app/command.go
+++ b/server/channels/app/command.go
@ -497,11 +497,23 @@ 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)
+	}
+	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 {
 		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)
 }
--- 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)

--- a/server/i18n/en.json
+++ b/server/i18n/en.json
@ -687,10 +687,18 @@
    "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"
  },
+  {
+    "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."