From 06c6ee2566003895bc9491241c7225b0cf891cb4 Mon Sep 17 00:00:00 2001 From: Mattermost Build Date: Tue, 20 Jan 2026 12:54:28 +0200 Subject: [PATCH] [MM-66789] Restrict ImportSettings.Directory changes via API and add validation (#34653) (#34987) Automatic Merge --- server/channels/api4/config.go | 9 +++ server/channels/api4/config_test.go | 61 ++++++++++++++++++++ server/cmd/mmctl/commands/config_e2e_test.go | 28 +++++++++ 3 files changed, 98 insertions(+) diff --git a/server/channels/api4/config.go b/server/channels/api4/config.go index 8c13da4b6f..40d8fbb099 100644 --- a/server/channels/api4/config.go +++ b/server/channels/api4/config.go @@ -159,6 +159,9 @@ func updateConfig(c *Context, w http.ResponseWriter, r *http.Request) { // modifications to the slice. cfg.PluginSettings.SignaturePublicKeyFiles = appCfg.PluginSettings.SignaturePublicKeyFiles + // Do not allow import directory to be changed through the API + *cfg.ImportSettings.Directory = *appCfg.ImportSettings.Directory + // Do not allow marketplace URL to be toggled through the API if EnableUploads are disabled. if cfg.PluginSettings.EnableUploads != nil && !*appCfg.PluginSettings.EnableUploads { *cfg.PluginSettings.MarketplaceURL = *appCfg.PluginSettings.MarketplaceURL @@ -317,6 +320,12 @@ func patchConfig(c *Context, w http.ResponseWriter, r *http.Request) { return } + // Do not allow import directory to be changed through the API + if cfg.ImportSettings.Directory != nil && *cfg.ImportSettings.Directory != *appCfg.ImportSettings.Directory { + c.Err = model.NewAppError("patchConfig", "api.config.update_config.not_allowed_security.app_error", map[string]any{"Name": "ImportSettings.Directory"}, "", http.StatusForbidden) + return + } + // Do not allow marketplace URL to be toggled if plugin uploads are disabled. if cfg.PluginSettings.MarketplaceURL != nil && cfg.PluginSettings.EnableUploads != nil { // Breaking it down to 2 conditions to make it simple. diff --git a/server/channels/api4/config_test.go b/server/channels/api4/config_test.go index cdba26fa54..6238191013 100644 --- a/server/channels/api4/config_test.go +++ b/server/channels/api4/config_test.go @@ -310,6 +310,43 @@ func TestUpdateConfig(t *testing.T) { CheckForbiddenStatus(t, resp) }) + t.Run("Should not be able to modify ImportSettings.Directory", func(t *testing.T) { + t.Run("sysadmin", func(t *testing.T) { + oldDirectory := *th.App.Config().ImportSettings.Directory + cfg2 := th.App.Config().Clone() + *cfg2.ImportSettings.Directory = "./new-import-dir" + + cfg2, _, err = th.SystemAdminClient.UpdateConfig(context.Background(), cfg2) + require.NoError(t, err) + assert.Equal(t, oldDirectory, *cfg2.ImportSettings.Directory) + assert.Equal(t, oldDirectory, *th.App.Config().ImportSettings.Directory) + + cfg2.ImportSettings.Directory = nil + cfg2, _, err = th.SystemAdminClient.UpdateConfig(context.Background(), cfg2) + require.NoError(t, err) + assert.Equal(t, oldDirectory, *cfg2.ImportSettings.Directory) + assert.Equal(t, oldDirectory, *th.App.Config().ImportSettings.Directory) + }) + + t.Run("local mode", func(t *testing.T) { + oldDirectory := *th.App.Config().ImportSettings.Directory + cfg2 := th.App.Config().Clone() + newDirectory := "./new-import-dir" + *cfg2.ImportSettings.Directory = newDirectory + + cfg2, _, err = th.LocalClient.UpdateConfig(context.Background(), cfg2) + require.NoError(t, err) + assert.Equal(t, newDirectory, *cfg2.ImportSettings.Directory) + assert.Equal(t, newDirectory, *th.App.Config().ImportSettings.Directory) + + cfg2.ImportSettings.Directory = nil + cfg2, _, err = th.LocalClient.UpdateConfig(context.Background(), cfg2) + require.NoError(t, err) + assert.Equal(t, oldDirectory, *cfg2.ImportSettings.Directory) + assert.Equal(t, oldDirectory, *th.App.Config().ImportSettings.Directory) + }) + }) + t.Run("System Admin should not be able to clear Site URL", func(t *testing.T) { siteURL := cfg.ServiceSettings.SiteURL defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.SiteURL = siteURL }) @@ -822,6 +859,30 @@ func TestPatchConfig(t *testing.T) { CheckForbiddenStatus(t, resp) } }) + + t.Run("not allowing to change import directory via api, unless local mode", func(t *testing.T) { + oldDirectory := *th.App.Config().ImportSettings.Directory + config := model.Config{ImportSettings: model.ImportSettings{ + Directory: model.NewPointer("./new-import-dir"), + }} + + updatedConfig, resp, err := client.PatchConfig(context.Background(), &config) + if client == th.LocalClient { + require.NoError(t, err) + CheckOKStatus(t, resp) + assert.Equal(t, "./new-import-dir", *updatedConfig.ImportSettings.Directory) + } else { + require.Error(t, err) + CheckForbiddenStatus(t, resp) + } + + // Reset for local mode + if client == th.LocalClient { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ImportSettings.Directory = oldDirectory + }) + } + }) }) t.Run("Should not be able to modify PluginSettings.MarketplaceURL if EnableUploads is disabled", func(t *testing.T) { diff --git a/server/cmd/mmctl/commands/config_e2e_test.go b/server/cmd/mmctl/commands/config_e2e_test.go index 044528ef00..f1ad94ce69 100644 --- a/server/cmd/mmctl/commands/config_e2e_test.go +++ b/server/cmd/mmctl/commands/config_e2e_test.go @@ -163,6 +163,34 @@ func (s *MmctlE2ETestSuite) TestConfigSetCmd() { s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 0) }) + + s.Run("ImportSettings.Directory cannot be set via API but can via local mode", func() { + printer.Clean() + originalDir := *s.th.App.Config().ImportSettings.Directory + + args := []string{"ImportSettings.Directory", "./api-blocked-import"} + err := configSetCmdF(s.th.SystemAdminClient, &cobra.Command{}, args) + s.Require().NotNil(err) + s.Require().Contains(err.Error(), "not allowed due to security reasons") + s.Require().Len(printer.GetLines(), 0) + + // Verify value didn't change + s.Require().Equal(originalDir, *s.th.App.Config().ImportSettings.Directory) + + printer.Clean() + args = []string{"ImportSettings.Directory", "./local-allowed-import"} + err = configSetCmdF(s.th.LocalClient, &cobra.Command{}, args) + s.Require().Nil(err) + s.Require().Len(printer.GetLines(), 1) + config, ok := printer.GetLines()[0].(*model.Config) + s.Require().True(ok) + s.Require().Equal("./local-allowed-import", *config.ImportSettings.Directory) + + // Reset to original + s.th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ImportSettings.Directory = &originalDir + }) + }) } func (s *MmctlE2ETestSuite) TestConfigEditCmd() {