From afd3c4e25a3acdf274270e2f307e5cc3644603ab Mon Sep 17 00:00:00 2001 From: Wayne Wollesen Date: Sat, 21 Mar 2026 20:21:57 -0700 Subject: [PATCH 1/8] Add mmctl config list command with config history and deltas Add a new `mmctl config list` subcommand that lists recent configuration entries from the database-backed config store. This provides visibility into config change history that was previously only accessible via direct database queries. Features: - Default mode shows changed setting paths per config entry - --detailed flag shows old and new values for each change - --no-delta flag shows only config metadata (id, timestamp, status) - --limit flag controls number of entries (default 5, max 100) - --json flag outputs structured JSON with embedded changes array - Sensitive fields display as [redacted] in detailed mode - Complex values (maps/slices) render as JSON - SetDefaults() applied before diffing to suppress schema evolution noise Full stack: model types, database query, config store, platform service, app layer, API endpoint (GET /config/list), Client4, mmctl command with unit tests. Co-Authored-By: Claude Opus 4.6 (1M context) --- server/channels/api4/config.go | 33 ++++++ server/channels/api4/config_local.go | 28 +++++ server/channels/app/config.go | 9 ++ server/channels/app/platform/config.go | 4 + server/cmd/mmctl/client/client.go | 1 + server/cmd/mmctl/commands/config.go | 102 +++++++++++++++++ server/cmd/mmctl/commands/config_test.go | 135 +++++++++++++++++++++++ server/cmd/mmctl/mocks/client_mock.go | 48 +++++--- server/cmd/mmctl/printer/printer.go | 5 + server/config/database.go | 122 ++++++++++++++++++++ server/config/store.go | 11 ++ server/i18n/en.json | 4 + server/public/model/audit_events.go | 1 + server/public/model/client4.go | 17 +++ server/public/model/config.go | 15 +++ 15 files changed, 519 insertions(+), 16 deletions(-) diff --git a/server/channels/api4/config.go b/server/channels/api4/config.go index c35ec4daa86..cfd59c6862b 100644 --- a/server/channels/api4/config.go +++ b/server/channels/api4/config.go @@ -38,6 +38,7 @@ func (api *API) InitConfig() { api.BaseRoutes.APIRoot.Handle("/config/reload", api.APISessionRequired(configReload)).Methods(http.MethodPost) api.BaseRoutes.APIRoot.Handle("/config/client", api.APIHandler(getClientConfig)).Methods(http.MethodGet) api.BaseRoutes.APIRoot.Handle("/config/environment", api.APISessionRequired(getEnvironmentConfig)).Methods(http.MethodGet) + api.BaseRoutes.APIRoot.Handle("/config/list", api.APISessionRequired(listConfigurations)).Methods(http.MethodGet) } func init() { @@ -458,3 +459,35 @@ func makeFilterConfigByPermission(accessType filterType) func(c *Context, struct return c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) } } + +func listConfigurations(c *Context, w http.ResponseWriter, r *http.Request) { + if !c.App.SessionHasPermissionToAny(*c.AppContext.Session(), model.SysconsoleReadPermissions) { + c.SetPermissionError(model.SysconsoleReadPermissions...) + return + } + + auditRec := c.MakeAuditRecord(model.AuditEventListConfigurations, model.AuditStatusFail) + defer c.LogAuditRec(auditRec) + + limit, _ := strconv.Atoi(r.URL.Query().Get("limit")) + if limit <= 0 { + limit = 5 + } + if limit > 100 { + limit = 100 + } + + includeDiffs := r.URL.Query().Get("include_diffs") + + items, appErr := c.App.ListConfigurations(limit, includeDiffs) + if appErr != nil { + c.Err = appErr + return + } + + auditRec.Success() + w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") + if err := json.NewEncoder(w).Encode(items); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} diff --git a/server/channels/api4/config_local.go b/server/channels/api4/config_local.go index c6a64357fb8..4a9dd6cc166 100644 --- a/server/channels/api4/config_local.go +++ b/server/channels/api4/config_local.go @@ -22,6 +22,7 @@ func (api *API) InitConfigLocal() { api.BaseRoutes.APIRoot.Handle("/config/reload", api.APILocal(configReload)).Methods(http.MethodPost) api.BaseRoutes.APIRoot.Handle("/config/migrate", api.APILocal(localMigrateConfig)).Methods(http.MethodPost) api.BaseRoutes.APIRoot.Handle("/config/client", api.APILocal(localGetClientConfig)).Methods(http.MethodGet) + api.BaseRoutes.APIRoot.Handle("/config/list", api.APILocal(localListConfigurations)).Methods(http.MethodGet) } func localGetConfig(c *Context, w http.ResponseWriter, r *http.Request) { @@ -199,3 +200,30 @@ func localGetClientConfig(c *Context, w http.ResponseWriter, r *http.Request) { c.Logger.Warn("Error while writing response", mlog.Err(err)) } } + +func localListConfigurations(c *Context, w http.ResponseWriter, r *http.Request) { + auditRec := c.MakeAuditRecord(model.AuditEventListConfigurations, model.AuditStatusFail) + defer c.LogAuditRec(auditRec) + + limit, _ := strconv.Atoi(r.URL.Query().Get("limit")) + if limit <= 0 { + limit = 5 + } + if limit > 100 { + limit = 100 + } + + includeDiffs := r.URL.Query().Get("include_diffs") + + items, appErr := c.App.ListConfigurations(limit, includeDiffs) + if appErr != nil { + c.Err = appErr + return + } + + auditRec.Success() + w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") + if err := json.NewEncoder(w).Encode(items); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} diff --git a/server/channels/app/config.go b/server/channels/app/config.go index 8eda0c3c287..73133bf3176 100644 --- a/server/channels/app/config.go +++ b/server/channels/app/config.go @@ -7,6 +7,7 @@ import ( "crypto/ecdsa" "crypto/rand" "encoding/json" + "net/http" "net/url" "reflect" "strconv" @@ -244,6 +245,14 @@ func (a *App) SaveConfig(newCfg *model.Config, sendConfigChangeClusterMessage bo return a.Srv().platform.SaveConfig(newCfg, sendConfigChangeClusterMessage) } +func (a *App) ListConfigurations(limit int, includeDiffs string) ([]*model.ConfigListItem, *model.AppError) { + items, err := a.Srv().platform.ListConfigurations(limit, includeDiffs) + if err != nil { + return nil, model.NewAppError("ListConfigurations", "api.config.list_configurations.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + return items, nil +} + func (a *App) HandleMessageExportConfig(cfg *model.Config, appCfg *model.Config) { // If the Message Export feature has been toggled in the System Console, rewrite the ExportFromTimestamp field to an // appropriate value. The rewriting occurs here to ensure it doesn't affect values written to the config file diff --git a/server/channels/app/platform/config.go b/server/channels/app/platform/config.go index 562d420091c..d6a4de57ca9 100644 --- a/server/channels/app/platform/config.go +++ b/server/channels/app/platform/config.go @@ -151,6 +151,10 @@ func (ps *PlatformService) CleanUpConfig() error { return ps.configStore.CleanUp() } +func (ps *PlatformService) ListConfigurations(limit int, includeDiffs string) ([]*model.ConfigListItem, error) { + return ps.configStore.ListConfigurations(limit, includeDiffs) +} + // ConfigureLogger applies the specified configuration to a logger. func (ps *PlatformService) ConfigureLogger(name string, logger *mlog.Logger, logSettings *model.LogSettings, getPath func(string) string) error { // Advanced logging is E20 only, however logging must be initialized before the license diff --git a/server/cmd/mmctl/client/client.go b/server/cmd/mmctl/client/client.go index 4f157884068..d78f580d00f 100644 --- a/server/cmd/mmctl/client/client.go +++ b/server/cmd/mmctl/client/client.go @@ -102,6 +102,7 @@ type Client interface { PatchConfig(context.Context, *model.Config) (*model.Config, *model.Response, error) ReloadConfig(ctx context.Context) (*model.Response, error) MigrateConfig(ctx context.Context, from, to string) (*model.Response, error) + ListConfigurations(ctx context.Context, limit int, includeDiffs string) ([]*model.ConfigListItem, *model.Response, error) SyncLdap(ctx context.Context) (*model.Response, error) MigrateIdLdap(ctx context.Context, toAttribute string) (*model.Response, error) GetUsers(ctx context.Context, page, perPage int, etag string) ([]*model.User, *model.Response, error) diff --git a/server/cmd/mmctl/commands/config.go b/server/cmd/mmctl/commands/config.go index b522eb5c3f7..2786466edcd 100644 --- a/server/cmd/mmctl/commands/config.go +++ b/server/cmd/mmctl/commands/config.go @@ -12,6 +12,7 @@ import ( "reflect" "strconv" "strings" + "time" "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/v8/channels/utils" @@ -129,6 +130,15 @@ var ConfigExportCmd = &cobra.Command{ RunE: withClient(configExportCmdF), } +var ConfigListCmd = &cobra.Command{ + Use: "list", + Short: "List stored configurations", + Long: "Lists recent configuration entries from the database-backed config store, showing Id, creation time, and active status.", + Example: " config list\n config list --limit 10", + Args: cobra.NoArgs, + RunE: withClient(configListCmdF), +} + func init() { ConfigResetCmd.Flags().Bool("confirm", false, "confirm you really want to reset all configuration settings to its default value") @@ -140,6 +150,10 @@ func init() { ConfigExportCmd.Flags().Bool("remove-masked", true, "remove masked values from the exported configuration") ConfigExportCmd.Flags().Bool("remove-defaults", false, "remove default values from the exported configuration") + ConfigListCmd.Flags().Int("limit", 5, "Maximum number of configurations to list") + ConfigListCmd.Flags().Bool("detailed", false, "Show old and new values for each change") + ConfigListCmd.Flags().Bool("no-delta", false, "Skip delta computation, show only config metadata") + ConfigCmd.AddCommand( ConfigGetCmd, ConfigSetCmd, @@ -151,6 +165,7 @@ func init() { ConfigMigrateCmd, ConfigSubpathCmd, ConfigExportCmd, + ConfigListCmd, ) RootCmd.AddCommand(ConfigCmd) } @@ -613,3 +628,90 @@ func configExportCmdF(c client.Client, cmd *cobra.Command, _ []string) error { return nil } + +func configListCmdF(c client.Client, cmd *cobra.Command, args []string) error { + limit, _ := cmd.Flags().GetInt("limit") + detailed, _ := cmd.Flags().GetBool("detailed") + noDelta, _ := cmd.Flags().GetBool("no-delta") + + includeDiffs := "true" + if detailed { + includeDiffs = "detailed" + } + if noDelta { + includeDiffs = "" + } + + items, _, err := c.ListConfigurations(context.TODO(), limit, includeDiffs) + if err != nil { + return fmt.Errorf("unable to list configurations: %w", err) + } + + if len(items) == 0 { + printer.Print("No configurations found. This feature requires a database-backed config store.") + return nil + } + + // "%-26s %s %-8s " = Id(26) + 2 + date(20) + 2 + status(8) + 2 + const prefix = "%-26s %s %-8s" + const padWidth = 26 + 2 + 20 + 2 + 8 + 2 + pad := strings.Repeat(" ", padWidth) + + for _, item := range items { + created := time.UnixMilli(item.CreateAt).UTC().Format(time.RFC3339) + status := "inactive" + if item.Active { + status = "active" + } + header := fmt.Sprintf(prefix, item.Id, created, status) + + if printer.GetFormat() != printer.FormatJSON && len(item.Changes) > 0 { + for i, change := range item.Changes { + var changeLine string + if detailed { + oldStr := formatConfigValue(change.OldValue) + newStr := formatConfigValue(change.NewValue) + if oldStr == model.FakeSetting || newStr == model.FakeSetting { + changeLine = fmt.Sprintf("%s: [redacted]", change.Path) + } else { + changeLine = fmt.Sprintf("%s: %s -> %s", change.Path, oldStr, newStr) + } + } else { + changeLine = change.Path + } + if i == 0 { + printer.PrintT(header+" "+changeLine, item) + } else { + printer.Print(pad + changeLine) + } + } + } else { + printer.PrintT(header, item) + } + } + + return nil +} + +func formatConfigValue(v any) string { + if v == nil { + return "" + } + rv := reflect.ValueOf(v) + switch rv.Kind() { + case reflect.Map, reflect.Slice: + b, err := json.Marshal(v) + if err != nil { + return fmt.Sprintf("%v", v) + } + return string(b) + case reflect.Float32, reflect.Float64: + f := rv.Float() + if f == float64(int64(f)) { + return strconv.FormatInt(int64(f), 10) + } + return strconv.FormatFloat(f, 'f', -1, 64) + default: + return fmt.Sprintf("%v", v) + } +} diff --git a/server/cmd/mmctl/commands/config_test.go b/server/cmd/mmctl/commands/config_test.go index a63dcf9011b..b0ceb9c540c 100644 --- a/server/cmd/mmctl/commands/config_test.go +++ b/server/cmd/mmctl/commands/config_test.go @@ -1025,3 +1025,138 @@ func (s *MmctlUnitTestSuite) TestConfigExportCmd() { s.Require().Len(printer.GetErrorLines(), 0) }) } + +func (s *MmctlUnitTestSuite) TestConfigListCmd() { + newListCmd := func() *cobra.Command { + cmd := &cobra.Command{} + cmd.Flags().Int("limit", 5, "") + cmd.Flags().Bool("detailed", false, "") + cmd.Flags().Bool("no-delta", false, "") + return cmd + } + + s.Run("List configurations with deltas by default", func() { + printer.Clean() + items := []*model.ConfigListItem{ + { + Id: "abc123def456ghi789jkl0mn", CreateAt: 1700000000000, Active: true, + Changes: []model.ConfigChange{{Path: "ServiceSettings.SiteURL"}}, + }, + {Id: "xyz789abc012def345ghi6mn", CreateAt: 1699999000000, Active: false}, + } + + s.client. + EXPECT(). + ListConfigurations(context.TODO(), 5, "true"). + Return(items, &model.Response{}, nil). + Times(1) + + err := configListCmdF(s.client, newListCmd(), []string{}) + s.Require().Nil(err) + // In JSON mode (test default), changes are embedded in struct, not separate lines + s.Require().Len(printer.GetLines(), 2) + s.Require().Len(printer.GetErrorLines(), 0) + // Verify the changes are embedded in the first item + item := printer.GetLines()[0].(*model.ConfigListItem) + s.Require().Len(item.Changes, 1) + s.Require().Equal("ServiceSettings.SiteURL", item.Changes[0].Path) + }) + + s.Run("List configurations with custom limit", func() { + printer.Clean() + items := []*model.ConfigListItem{ + {Id: "abc123def456ghi789jkl0mn", CreateAt: 1700000000000, Active: true}, + } + + s.client. + EXPECT(). + ListConfigurations(context.TODO(), 10, "true"). + Return(items, &model.Response{}, nil). + Times(1) + + cmd := newListCmd() + _ = cmd.Flags().Set("limit", "10") + + err := configListCmdF(s.client, cmd, []string{}) + s.Require().Nil(err) + s.Require().Len(printer.GetLines(), 1) + }) + + s.Run("List configurations returns error", func() { + printer.Clean() + + s.client. + EXPECT(). + ListConfigurations(context.TODO(), 5, "true"). + Return(nil, &model.Response{}, errors.New("database error")). + Times(1) + + err := configListCmdF(s.client, newListCmd(), []string{}) + s.Require().NotNil(err) + s.Require().Contains(err.Error(), "unable to list configurations") + }) + + s.Run("No configurations found", func() { + printer.Clean() + + s.client. + EXPECT(). + ListConfigurations(context.TODO(), 5, "true"). + Return([]*model.ConfigListItem{}, &model.Response{}, nil). + Times(1) + + err := configListCmdF(s.client, newListCmd(), []string{}) + s.Require().Nil(err) + s.Require().Len(printer.GetLines(), 1) + }) + + s.Run("Detailed mode shows old and new values", func() { + printer.Clean() + items := []*model.ConfigListItem{ + { + Id: "abc123def456ghi789jkl0mn", CreateAt: 1700000000000, Active: true, + Changes: []model.ConfigChange{ + {Path: "ServiceSettings.SiteURL", OldValue: "", NewValue: "https://mm.example.com"}, + }, + }, + } + + s.client. + EXPECT(). + ListConfigurations(context.TODO(), 5, "detailed"). + Return(items, &model.Response{}, nil). + Times(1) + + cmd := newListCmd() + _ = cmd.Flags().Set("detailed", "true") + + err := configListCmdF(s.client, cmd, []string{}) + s.Require().Nil(err) + // In JSON mode, changes embedded in struct + s.Require().Len(printer.GetLines(), 1) + item := printer.GetLines()[0].(*model.ConfigListItem) + s.Require().Len(item.Changes, 1) + s.Require().Equal("https://mm.example.com", item.Changes[0].NewValue) + }) + + s.Run("No-delta mode skips diffs", func() { + printer.Clean() + items := []*model.ConfigListItem{ + {Id: "abc123def456ghi789jkl0mn", CreateAt: 1700000000000, Active: true}, + {Id: "xyz789abc012def345ghi6mn", CreateAt: 1699999000000, Active: false}, + } + + s.client. + EXPECT(). + ListConfigurations(context.TODO(), 5, ""). + Return(items, &model.Response{}, nil). + Times(1) + + cmd := newListCmd() + _ = cmd.Flags().Set("no-delta", "true") + + err := configListCmdF(s.client, cmd, []string{}) + s.Require().Nil(err) + s.Require().Len(printer.GetLines(), 2) + }) +} diff --git a/server/cmd/mmctl/mocks/client_mock.go b/server/cmd/mmctl/mocks/client_mock.go index 3003fff8b3f..0c9b7a98522 100644 --- a/server/cmd/mmctl/mocks/client_mock.go +++ b/server/cmd/mmctl/mocks/client_mock.go @@ -650,6 +650,22 @@ func (mr *MockClientMockRecorder) GenerateSupportPacket(arg0 interface{}) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateSupportPacket", reflect.TypeOf((*MockClient)(nil).GenerateSupportPacket), arg0) } +// GetAllRoles mocks base method. +func (m *MockClient) GetAllRoles(arg0 context.Context) ([]*model.Role, *model.Response, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAllRoles", arg0) + ret0, _ := ret[0].([]*model.Role) + ret1, _ := ret[1].(*model.Response) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// GetAllRoles indicates an expected call of GetAllRoles. +func (mr *MockClientMockRecorder) GetAllRoles(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllRoles", reflect.TypeOf((*MockClient)(nil).GetAllRoles), arg0) +} + // GetAllTeams mocks base method. func (m *MockClient) GetAllTeams(arg0 context.Context, arg1 string, arg2, arg3 int) ([]*model.Team, *model.Response, error) { m.ctrl.T.Helper() @@ -1372,22 +1388,6 @@ func (mr *MockClientMockRecorder) GetRoleByName(arg0, arg1 interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRoleByName", reflect.TypeOf((*MockClient)(nil).GetRoleByName), arg0, arg1) } -// GetAllRoles mocks base method. -func (m *MockClient) GetAllRoles(arg0 context.Context) ([]*model.Role, *model.Response, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetAllRoles", arg0) - ret0, _ := ret[0].([]*model.Role) - ret1, _ := ret[1].(*model.Response) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 -} - -// GetAllRoles indicates an expected call of GetAllRoles. -func (mr *MockClientMockRecorder) GetAllRoles(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllRoles", reflect.TypeOf((*MockClient)(nil).GetAllRoles), arg0) -} - // GetServerBusy mocks base method. func (m *MockClient) GetServerBusy(arg0 context.Context) (*model.ServerBusyState, *model.Response, error) { m.ctrl.T.Helper() @@ -1691,6 +1691,22 @@ func (mr *MockClientMockRecorder) ListCommands(arg0, arg1, arg2 interface{}) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListCommands", reflect.TypeOf((*MockClient)(nil).ListCommands), arg0, arg1, arg2) } +// ListConfigurations mocks base method. +func (m *MockClient) ListConfigurations(arg0 context.Context, arg1 int, arg2 string) ([]*model.ConfigListItem, *model.Response, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListConfigurations", arg0, arg1, arg2) + ret0, _ := ret[0].([]*model.ConfigListItem) + ret1, _ := ret[1].(*model.Response) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// ListConfigurations indicates an expected call of ListConfigurations. +func (mr *MockClientMockRecorder) ListConfigurations(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListConfigurations", reflect.TypeOf((*MockClient)(nil).ListConfigurations), arg0, arg1, arg2) +} + // ListExports mocks base method. func (m *MockClient) ListExports(arg0 context.Context) ([]string, *model.Response, error) { m.ctrl.T.Helper() diff --git a/server/cmd/mmctl/printer/printer.go b/server/cmd/mmctl/printer/printer.go index db199bb58cb..f85b10e9b0c 100644 --- a/server/cmd/mmctl/printer/printer.go +++ b/server/cmd/mmctl/printer/printer.go @@ -62,6 +62,11 @@ func SetFormat(t string) { printer.Format = t } +// GetFormat returns the current output format +func GetFormat() string { + return printer.Format +} + func SetCommand(cmd *cobra.Command) { printer.cmd = cmd } diff --git a/server/config/database.go b/server/config/database.go index 7708b43eb52..13790d1dc6e 100644 --- a/server/config/database.go +++ b/server/config/database.go @@ -13,6 +13,7 @@ import ( "encoding/json" "fmt" "path/filepath" + "sort" "strings" "github.com/jmoiron/sqlx" @@ -350,3 +351,124 @@ func (ds *DatabaseStore) cleanUp(thresholdCreateAt int64) error { return nil } + +func (ds *DatabaseStore) listConfigurations(limit int, includeDiffs string) ([]*model.ConfigListItem, error) { + type configListRow struct { + Id string `db:"id"` + CreateAt int64 `db:"createat"` + Active bool `db:"active"` + } + + query := ` + SELECT Id, CreateAt, COALESCE(Active, false) AS Active + FROM Configurations + ORDER BY Active DESC NULLS LAST, CreateAt DESC + LIMIT $1 + ` + + var rows []configListRow + if err := ds.db.Select(&rows, query, limit); err != nil { + return nil, errors.Wrap(err, "failed to list configurations") + } + + items := make([]*model.ConfigListItem, len(rows)) + for i, row := range rows { + items[i] = &model.ConfigListItem{ + Id: row.Id, + CreateAt: row.CreateAt, + Active: row.Active, + } + } + + if includeDiffs == "" || len(items) <= 1 { + return items, nil + } + + // Fetch config values and compute diffs between consecutive entries + ids := make([]string, len(items)) + for i, item := range items { + ids[i] = item.Id + } + + values, err := ds.getConfigValuesByIDs(ids) + if err != nil { + return nil, err + } + + // Unmarshal configs and apply SetDefaults() so that diffs only reflect + // intentional user changes, not new fields introduced by server upgrades. + // Without SetDefaults(), upgrading the server would cause every new config + // field to appear as a diff (nil -> default value), which is noise. + configs := make(map[string]*model.Config, len(values)) + for id, raw := range values { + cfg := &model.Config{} + if err := json.Unmarshal(raw, cfg); err != nil { + return nil, errors.Wrapf(err, "failed to unmarshal config %s", id) + } + cfg.SetDefaults() + configs[id] = cfg + } + + // Sort chronologically (oldest first) for diffing + sorted := make([]*model.ConfigListItem, len(items)) + copy(sorted, items) + sort.Slice(sorted, func(i, j int) bool { + return sorted[i].CreateAt < sorted[j].CreateAt + }) + + detailed := includeDiffs == "detailed" + changesMap := make(map[string][]model.ConfigChange) + for i := 1; i < len(sorted); i++ { + prevCfg := configs[sorted[i-1].Id] + currCfg := configs[sorted[i].Id] + if prevCfg == nil || currCfg == nil { + continue + } + diffs, diffErr := Diff(prevCfg, currCfg) + if diffErr != nil { + continue + } + diffs = diffs.Sanitize() + + changes := make([]model.ConfigChange, len(diffs)) + for j, d := range diffs { + changes[j] = model.ConfigChange{Path: d.Path} + if detailed { + changes[j].OldValue = d.BaseVal + changes[j].NewValue = d.ActualVal + } + } + changesMap[sorted[i].Id] = changes + } + + for _, item := range items { + if c, ok := changesMap[item.Id]; ok { + item.Changes = c + } + } + + return items, nil +} + +func (ds *DatabaseStore) getConfigValuesByIDs(ids []string) (map[string][]byte, error) { + query, args, err := sqlx.In("SELECT Id, Value FROM Configurations WHERE Id IN (?)", ids) + if err != nil { + return nil, errors.Wrap(err, "failed to build query") + } + query = ds.db.Rebind(query) + + type row struct { + Id string `db:"id"` + Value []byte `db:"value"` + } + var rows []row + if err := ds.db.Select(&rows, query, args...); err != nil { + return nil, errors.Wrap(err, "failed to fetch configuration values") + } + + result := make(map[string][]byte, len(rows)) + for _, r := range rows { + result[r.Id] = r.Value + } + return result, nil +} diff --git a/server/config/store.go b/server/config/store.go index 66083456eeb..7ba896773a8 100644 --- a/server/config/store.go +++ b/server/config/store.go @@ -411,3 +411,14 @@ func (s *Store) CleanUp() error { return nil } } + +// ListConfigurations returns metadata for recent configurations. +// This only works with DatabaseStore; returns an error for FileStore. +func (s *Store) ListConfigurations(limit int, includeDiffs string) ([]*model.ConfigListItem, error) { + switch bs := s.backingStore.(type) { + case *DatabaseStore: + return bs.listConfigurations(limit, includeDiffs) + default: + return nil, errors.New("listing configurations is only supported with database-backed config stores") + } +} diff --git a/server/i18n/en.json b/server/i18n/en.json index 503c4618df6..0b6df23957d 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -1745,6 +1745,10 @@ "id": "api.config.get_config.restricted_merge.app_error", "translation": "Failed to merge given config." }, + { + "id": "api.config.list_configurations.app_error", + "translation": "Unable to list configurations." + }, { "id": "api.config.migrate_config.app_error", "translation": "Failed to migrate config store." diff --git a/server/public/model/audit_events.go b/server/public/model/audit_events.go index f9fcb628581..904c519671e 100644 --- a/server/public/model/audit_events.go +++ b/server/public/model/audit_events.go @@ -110,6 +110,7 @@ const ( const ( AuditEventConfigReload = "configReload" // reload server configuration AuditEventGetConfig = "getConfig" // get current server configuration + AuditEventListConfigurations = "listConfigurations" // list stored configuration history AuditEventLocalGetClientConfig = "localGetClientConfig" // get client configuration locally AuditEventLocalGetConfig = "localGetConfig" // get server configuration locally AuditEventLocalPatchConfig = "localPatchConfig" // update server configuration locally diff --git a/server/public/model/client4.go b/server/public/model/client4.go index f03e08bee01..b4404290a85 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -4307,6 +4307,23 @@ func (c *Client4) ReloadConfig(ctx context.Context) (*Response, error) { return BuildResponse(r), nil } +// ListConfigurations returns metadata for stored configuration entries. +func (c *Client4) ListConfigurations(ctx context.Context, limit int, includeDiffs string) ([]*ConfigListItem, *Response, error) { + query := url.Values{} + if limit > 0 { + query.Set("limit", strconv.Itoa(limit)) + } + if includeDiffs != "" { + query.Set("include_diffs", includeDiffs) + } + r, err := c.doAPIGetWithQuery(ctx, c.configRoute().Join("list"), query, "") + if err != nil { + return nil, BuildResponse(r), err + } + defer closeBody(r) + return DecodeJSONFromResponse[[]*ConfigListItem](r) +} + // GetClientConfig will retrieve the parts of the server configuration needed by the client. func (c *Client4) GetClientConfig(ctx context.Context, etag string) (map[string]string, *Response, error) { r, err := c.doAPIGet(ctx, c.configRoute().Join("client"), etag) diff --git a/server/public/model/config.go b/server/public/model/config.go index 2f4eb7a0b01..0bb04236c1b 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -5148,6 +5148,21 @@ type GetConfigOptions struct { RemoveDefaults bool } +// ConfigChange represents a single setting change between two configuration versions. +type ConfigChange struct { + Path string `json:"path"` + OldValue any `json:"old_value,omitempty"` + NewValue any `json:"new_value,omitempty"` +} + +// ConfigListItem represents metadata about a stored configuration entry. +type ConfigListItem struct { + Id string `json:"id"` + CreateAt int64 `json:"create_at"` + Active bool `json:"active"` + Changes []ConfigChange `json:"changes,omitempty"` +} + // FilterConfig returns a map[string]any representation of the configuration. // Also, the function can filter the configuration by the options passed // in the argument. The options are used to remove the default values, the masked From 1dd00462579358ee220d8b074b289869c046b925 Mon Sep 17 00:00:00 2001 From: Wayne Wollesen Date: Sat, 21 Mar 2026 20:45:20 -0700 Subject: [PATCH 2/8] Add mmctl config rollback command Add `mmctl config rollback ` to restore a previous configuration by ID. The command fetches the historical config from the Configurations table and passes it through the existing SaveConfig flow, which handles SetDefaults, password restoration, validation, atomic DB swap, and cluster notifications. A new config entry is created rather than reactivating the old row, preserving the full audit trail. Use `mmctl config list` to find available config IDs to rollback to. Full stack: getConfigByID database query, config store, platform service, app layer RollbackConfig, API endpoint (POST /config/rollback), Client4, mmctl command with unit tests. Co-Authored-By: Claude Opus 4.6 (1M context) --- server/channels/api4/config.go | 33 ++++++++++++++++++++++++ server/channels/api4/config_local.go | 28 ++++++++++++++++++++ server/channels/app/config.go | 8 ++++++ server/channels/app/platform/config.go | 4 +++ server/cmd/mmctl/client/client.go | 1 + server/cmd/mmctl/commands/config.go | 21 +++++++++++++++ server/cmd/mmctl/commands/config_test.go | 33 ++++++++++++++++++++++++ server/cmd/mmctl/mocks/client_mock.go | 16 ++++++++++++ server/config/database.go | 13 ++++++++++ server/config/store.go | 11 ++++++++ server/i18n/en.json | 8 ++++++ server/public/model/audit_events.go | 1 + server/public/model/client4.go | 11 ++++++++ 13 files changed, 188 insertions(+) diff --git a/server/channels/api4/config.go b/server/channels/api4/config.go index cfd59c6862b..23b44d0d4a9 100644 --- a/server/channels/api4/config.go +++ b/server/channels/api4/config.go @@ -39,6 +39,7 @@ func (api *API) InitConfig() { api.BaseRoutes.APIRoot.Handle("/config/client", api.APIHandler(getClientConfig)).Methods(http.MethodGet) api.BaseRoutes.APIRoot.Handle("/config/environment", api.APISessionRequired(getEnvironmentConfig)).Methods(http.MethodGet) api.BaseRoutes.APIRoot.Handle("/config/list", api.APISessionRequired(listConfigurations)).Methods(http.MethodGet) + api.BaseRoutes.APIRoot.Handle("/config/rollback", api.APISessionRequired(rollbackConfig)).Methods(http.MethodPost) } func init() { @@ -491,3 +492,35 @@ func listConfigurations(c *Context, w http.ResponseWriter, r *http.Request) { c.Logger.Warn("Error while writing response", mlog.Err(err)) } } + +func rollbackConfig(c *Context, w http.ResponseWriter, r *http.Request) { + if !c.App.SessionHasPermissionToAny(*c.AppContext.Session(), model.SysconsoleWritePermissions) { + c.SetPermissionError(model.SysconsoleWritePermissions...) + return + } + + auditRec := c.MakeAuditRecord(model.AuditEventRollbackConfig, model.AuditStatusFail) + defer c.LogAuditRec(auditRec) + + var body struct { + ConfigID string `json:"config_id"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil || body.ConfigID == "" { + c.SetInvalidParamWithErr("config_id", err) + return + } + + _, newCfg, appErr := c.App.RollbackConfig(body.ConfigID) + if appErr != nil { + c.Err = appErr + return + } + + c.App.SanitizedConfig(newCfg) + + auditRec.Success() + w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") + if err := json.NewEncoder(w).Encode(newCfg); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} diff --git a/server/channels/api4/config_local.go b/server/channels/api4/config_local.go index 4a9dd6cc166..6b8c683c404 100644 --- a/server/channels/api4/config_local.go +++ b/server/channels/api4/config_local.go @@ -23,6 +23,7 @@ func (api *API) InitConfigLocal() { api.BaseRoutes.APIRoot.Handle("/config/migrate", api.APILocal(localMigrateConfig)).Methods(http.MethodPost) api.BaseRoutes.APIRoot.Handle("/config/client", api.APILocal(localGetClientConfig)).Methods(http.MethodGet) api.BaseRoutes.APIRoot.Handle("/config/list", api.APILocal(localListConfigurations)).Methods(http.MethodGet) + api.BaseRoutes.APIRoot.Handle("/config/rollback", api.APILocal(localRollbackConfig)).Methods(http.MethodPost) } func localGetConfig(c *Context, w http.ResponseWriter, r *http.Request) { @@ -227,3 +228,30 @@ func localListConfigurations(c *Context, w http.ResponseWriter, r *http.Request) c.Logger.Warn("Error while writing response", mlog.Err(err)) } } + +func localRollbackConfig(c *Context, w http.ResponseWriter, r *http.Request) { + auditRec := c.MakeAuditRecord(model.AuditEventRollbackConfig, model.AuditStatusFail) + defer c.LogAuditRec(auditRec) + + var body struct { + ConfigID string `json:"config_id"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil || body.ConfigID == "" { + c.SetInvalidParamWithErr("config_id", err) + return + } + + _, newCfg, appErr := c.App.RollbackConfig(body.ConfigID) + if appErr != nil { + c.Err = appErr + return + } + + c.App.SanitizedConfig(newCfg) + + auditRec.Success() + w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") + if err := json.NewEncoder(w).Encode(newCfg); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} diff --git a/server/channels/app/config.go b/server/channels/app/config.go index 73133bf3176..42522322d61 100644 --- a/server/channels/app/config.go +++ b/server/channels/app/config.go @@ -253,6 +253,14 @@ func (a *App) ListConfigurations(limit int, includeDiffs string) ([]*model.Confi return items, nil } +func (a *App) RollbackConfig(id string) (*model.Config, *model.Config, *model.AppError) { + historicalCfg, err := a.Srv().platform.GetConfigByID(id) + if err != nil { + return nil, nil, model.NewAppError("RollbackConfig", "api.config.rollback_config.not_found.app_error", nil, "", http.StatusNotFound).Wrap(err) + } + return a.SaveConfig(historicalCfg, true) +} + func (a *App) HandleMessageExportConfig(cfg *model.Config, appCfg *model.Config) { // If the Message Export feature has been toggled in the System Console, rewrite the ExportFromTimestamp field to an // appropriate value. The rewriting occurs here to ensure it doesn't affect values written to the config file diff --git a/server/channels/app/platform/config.go b/server/channels/app/platform/config.go index d6a4de57ca9..0c3a5c9131c 100644 --- a/server/channels/app/platform/config.go +++ b/server/channels/app/platform/config.go @@ -155,6 +155,10 @@ func (ps *PlatformService) ListConfigurations(limit int, includeDiffs string) ([ return ps.configStore.ListConfigurations(limit, includeDiffs) } +func (ps *PlatformService) GetConfigByID(id string) (*model.Config, error) { + return ps.configStore.GetConfigByID(id) +} + // ConfigureLogger applies the specified configuration to a logger. func (ps *PlatformService) ConfigureLogger(name string, logger *mlog.Logger, logSettings *model.LogSettings, getPath func(string) string) error { // Advanced logging is E20 only, however logging must be initialized before the license diff --git a/server/cmd/mmctl/client/client.go b/server/cmd/mmctl/client/client.go index d78f580d00f..0ea35659c33 100644 --- a/server/cmd/mmctl/client/client.go +++ b/server/cmd/mmctl/client/client.go @@ -103,6 +103,7 @@ type Client interface { ReloadConfig(ctx context.Context) (*model.Response, error) MigrateConfig(ctx context.Context, from, to string) (*model.Response, error) ListConfigurations(ctx context.Context, limit int, includeDiffs string) ([]*model.ConfigListItem, *model.Response, error) + RollbackConfig(ctx context.Context, configID string) (*model.Config, *model.Response, error) SyncLdap(ctx context.Context) (*model.Response, error) MigrateIdLdap(ctx context.Context, toAttribute string) (*model.Response, error) GetUsers(ctx context.Context, page, perPage int, etag string) ([]*model.User, *model.Response, error) diff --git a/server/cmd/mmctl/commands/config.go b/server/cmd/mmctl/commands/config.go index 2786466edcd..9ba219da51e 100644 --- a/server/cmd/mmctl/commands/config.go +++ b/server/cmd/mmctl/commands/config.go @@ -139,6 +139,15 @@ var ConfigListCmd = &cobra.Command{ RunE: withClient(configListCmdF), } +var ConfigRollbackCmd = &cobra.Command{ + Use: "rollback [config_id]", + Short: "Rollback to a previous configuration", + Long: "Restores a previous configuration by ID. Use 'config list' to find available config IDs. This creates a new config entry preserving the full audit trail.", + Example: " config rollback abc123def456ghi789jkl0mn", + Args: cobra.ExactArgs(1), + RunE: withClient(configRollbackCmdF), +} + func init() { ConfigResetCmd.Flags().Bool("confirm", false, "confirm you really want to reset all configuration settings to its default value") @@ -166,6 +175,7 @@ func init() { ConfigSubpathCmd, ConfigExportCmd, ConfigListCmd, + ConfigRollbackCmd, ) RootCmd.AddCommand(ConfigCmd) } @@ -715,3 +725,14 @@ func formatConfigValue(v any) string { return fmt.Sprintf("%v", v) } } + +func configRollbackCmdF(c client.Client, cmd *cobra.Command, args []string) error { + _, _, err := c.RollbackConfig(context.TODO(), args[0]) + if err != nil { + return fmt.Errorf("unable to rollback configuration: %w", err) + } + + printer.Print(fmt.Sprintf("Configuration rolled back successfully to %s.", args[0])) + + return nil +} diff --git a/server/cmd/mmctl/commands/config_test.go b/server/cmd/mmctl/commands/config_test.go index b0ceb9c540c..c4e704ead51 100644 --- a/server/cmd/mmctl/commands/config_test.go +++ b/server/cmd/mmctl/commands/config_test.go @@ -1160,3 +1160,36 @@ func (s *MmctlUnitTestSuite) TestConfigListCmd() { s.Require().Len(printer.GetLines(), 2) }) } + +func (s *MmctlUnitTestSuite) TestConfigRollbackCmd() { + s.Run("Successful rollback", func() { + printer.Clean() + cfg := &model.Config{} + cfg.SetDefaults() + + s.client. + EXPECT(). + RollbackConfig(context.TODO(), "abc123def456ghi789jkl0mn"). + Return(cfg, &model.Response{}, nil). + Times(1) + + err := configRollbackCmdF(s.client, &cobra.Command{}, []string{"abc123def456ghi789jkl0mn"}) + s.Require().Nil(err) + s.Require().Len(printer.GetLines(), 1) + s.Require().Contains(printer.GetLines()[0], "rolled back successfully") + }) + + s.Run("Rollback returns error", func() { + printer.Clean() + + s.client. + EXPECT(). + RollbackConfig(context.TODO(), "nonexistent"). + Return(nil, &model.Response{}, errors.New("configuration not found")). + Times(1) + + err := configRollbackCmdF(s.client, &cobra.Command{}, []string{"nonexistent"}) + s.Require().NotNil(err) + s.Require().Contains(err.Error(), "unable to rollback configuration") + }) +} diff --git a/server/cmd/mmctl/mocks/client_mock.go b/server/cmd/mmctl/mocks/client_mock.go index 0c9b7a98522..752d6051614 100644 --- a/server/cmd/mmctl/mocks/client_mock.go +++ b/server/cmd/mmctl/mocks/client_mock.go @@ -2234,6 +2234,22 @@ func (mr *MockClientMockRecorder) RevokeUserAccessToken(arg0, arg1 interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RevokeUserAccessToken", reflect.TypeOf((*MockClient)(nil).RevokeUserAccessToken), arg0, arg1) } +// RollbackConfig mocks base method. +func (m *MockClient) RollbackConfig(arg0 context.Context, arg1 string) (*model.Config, *model.Response, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RollbackConfig", arg0, arg1) + ret0, _ := ret[0].(*model.Config) + ret1, _ := ret[1].(*model.Response) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// RollbackConfig indicates an expected call of RollbackConfig. +func (mr *MockClientMockRecorder) RollbackConfig(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RollbackConfig", reflect.TypeOf((*MockClient)(nil).RollbackConfig), arg0, arg1) +} + // SearchTeams mocks base method. func (m *MockClient) SearchTeams(arg0 context.Context, arg1 *model.TeamSearch) ([]*model.Team, *model.Response, error) { m.ctrl.T.Helper() diff --git a/server/config/database.go b/server/config/database.go index 13790d1dc6e..ca913b22cff 100644 --- a/server/config/database.go +++ b/server/config/database.go @@ -472,3 +472,16 @@ func (ds *DatabaseStore) getConfigValuesByIDs(ids []string) (map[string][]byte, } return result, nil } + +func (ds *DatabaseStore) getConfigByID(id string) (*model.Config, error) { + var value []byte + if err := ds.db.Get(&value, "SELECT Value FROM Configurations WHERE Id = $1", id); err != nil { + return nil, errors.Wrapf(err, "failed to fetch configuration %s", id) + } + + cfg := &model.Config{} + if err := json.Unmarshal(value, cfg); err != nil { + return nil, errors.Wrapf(err, "failed to unmarshal configuration %s", id) + } + return cfg, nil +} diff --git a/server/config/store.go b/server/config/store.go index 7ba896773a8..c372c9ee904 100644 --- a/server/config/store.go +++ b/server/config/store.go @@ -422,3 +422,14 @@ func (s *Store) ListConfigurations(limit int, includeDiffs string) ([]*model.Con return nil, errors.New("listing configurations is only supported with database-backed config stores") } } + +// GetConfigByID retrieves a historical configuration by its ID. +// This only works with DatabaseStore; returns an error for FileStore. +func (s *Store) GetConfigByID(id string) (*model.Config, error) { + switch bs := s.backingStore.(type) { + case *DatabaseStore: + return bs.getConfigByID(id) + default: + return nil, errors.New("retrieving configurations by ID is only supported with database-backed config stores") + } +} diff --git a/server/i18n/en.json b/server/i18n/en.json index 0b6df23957d..0ace9bddc8b 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -1749,6 +1749,14 @@ "id": "api.config.list_configurations.app_error", "translation": "Unable to list configurations." }, + { + "id": "api.config.rollback_config.app_error", + "translation": "Unable to rollback configuration." + }, + { + "id": "api.config.rollback_config.not_found.app_error", + "translation": "Configuration not found." + }, { "id": "api.config.migrate_config.app_error", "translation": "Failed to migrate config store." diff --git a/server/public/model/audit_events.go b/server/public/model/audit_events.go index 904c519671e..171a309425c 100644 --- a/server/public/model/audit_events.go +++ b/server/public/model/audit_events.go @@ -112,6 +112,7 @@ const ( AuditEventGetConfig = "getConfig" // get current server configuration AuditEventListConfigurations = "listConfigurations" // list stored configuration history AuditEventLocalGetClientConfig = "localGetClientConfig" // get client configuration locally + AuditEventRollbackConfig = "rollbackConfig" // rollback to a previous configuration AuditEventLocalGetConfig = "localGetConfig" // get server configuration locally AuditEventLocalPatchConfig = "localPatchConfig" // update server configuration locally AuditEventLocalUpdateConfig = "localUpdateConfig" // update server configuration locally diff --git a/server/public/model/client4.go b/server/public/model/client4.go index b4404290a85..df75476cff8 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -4324,6 +4324,17 @@ func (c *Client4) ListConfigurations(ctx context.Context, limit int, includeDiff return DecodeJSONFromResponse[[]*ConfigListItem](r) } +// RollbackConfig restores a historical configuration by ID. +func (c *Client4) RollbackConfig(ctx context.Context, configID string) (*Config, *Response, error) { + body := map[string]string{"config_id": configID} + r, err := c.doAPIPostJSON(ctx, c.configRoute().Join("rollback"), body) + if err != nil { + return nil, BuildResponse(r), err + } + defer closeBody(r) + return DecodeJSONFromResponse[*Config](r) +} + // GetClientConfig will retrieve the parts of the server configuration needed by the client. func (c *Client4) GetClientConfig(ctx context.Context, etag string) (map[string]string, *Response, error) { r, err := c.doAPIGet(ctx, c.configRoute().Join("client"), etag) From 42a1a50092946d60352debbc84d39bde91e2b96d Mon Sep 17 00:00:00 2001 From: Wayne Wollesen Date: Sat, 21 Mar 2026 21:31:58 -0700 Subject: [PATCH 3/8] Require manage_system permission for config list and rollback endpoints Tighten permissions from SysconsoleReadPermissions/SysconsoleWritePermissions to PermissionManageSystem (system_admin) for both the config list and rollback API endpoints. These are admin diagnostic tools that expose config history and shouldn't be accessible to users with partial sysconsole permissions. Co-Authored-By: Claude Opus 4.6 (1M context) --- server/channels/api4/config.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/channels/api4/config.go b/server/channels/api4/config.go index 23b44d0d4a9..02023e30f16 100644 --- a/server/channels/api4/config.go +++ b/server/channels/api4/config.go @@ -462,8 +462,8 @@ func makeFilterConfigByPermission(accessType filterType) func(c *Context, struct } func listConfigurations(c *Context, w http.ResponseWriter, r *http.Request) { - if !c.App.SessionHasPermissionToAny(*c.AppContext.Session(), model.SysconsoleReadPermissions) { - c.SetPermissionError(model.SysconsoleReadPermissions...) + if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) { + c.SetPermissionError(model.PermissionManageSystem) return } @@ -494,8 +494,8 @@ func listConfigurations(c *Context, w http.ResponseWriter, r *http.Request) { } func rollbackConfig(c *Context, w http.ResponseWriter, r *http.Request) { - if !c.App.SessionHasPermissionToAny(*c.AppContext.Session(), model.SysconsoleWritePermissions) { - c.SetPermissionError(model.SysconsoleWritePermissions...) + if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) { + c.SetPermissionError(model.PermissionManageSystem) return } From 8aa5feca81668955bc2b5f7cf3fe7696f4e72937 Mon Sep 17 00:00:00 2001 From: Wayne Wollesen Date: Sun, 22 Mar 2026 08:22:10 -0700 Subject: [PATCH 4/8] Add OpenAPI specs for config list/rollback and apply cloud filtering to rollback response - Add OpenAPI 3 definitions for GET /api/v4/config/list and POST /api/v4/config/rollback to fix vet-api CI failure - Apply cloud-restrictable field filtering to rollback config response, matching updateConfig and patchConfig behavior - Improve error handling in RollbackConfig to distinguish not-found from internal errors - Fix config list diff computation to include predecessor row for oldest visible item - Fix empty config list JSON output format Co-Authored-By: Claude Opus 4.6 (1M context) --- api/v4/source/system.yaml | 77 +++++++++++++++++++++++++++++ server/channels/api4/config.go | 12 +++++ server/channels/app/config.go | 7 ++- server/cmd/mmctl/commands/config.go | 6 ++- server/config/database.go | 36 ++++++++++---- 5 files changed, 126 insertions(+), 12 deletions(-) diff --git a/api/v4/source/system.yaml b/api/v4/source/system.yaml index cd0ab8a4d92..7c10e77998c 100644 --- a/api/v4/source/system.yaml +++ b/api/v4/source/system.yaml @@ -488,6 +488,83 @@ $ref: "#/components/responses/Unauthorized" "403": $ref: "#/components/responses/Forbidden" + /api/v4/config/list: + get: + tags: + - system + summary: List configuration history + description: | + Retrieve a list of previous configurations with optional diffs between them. + + ##### Permissions + Must have `manage_system` permission. + operationId: ListConfigurations + parameters: + - name: limit + in: query + description: The number of configuration history entries to return. Default is 5, maximum is 100. + required: false + schema: + type: integer + default: 5 + - name: include_diffs + in: query + description: Whether to include diffs between configurations. Set to "true" to include. + required: false + schema: + type: string + responses: + "200": + description: Configuration history retrieval successful + content: + application/json: + schema: + type: array + items: + type: object + "400": + $ref: "#/components/responses/BadRequest" + "401": + $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" + /api/v4/config/rollback: + post: + tags: + - system + summary: Rollback to a previous configuration + description: | + Rollback the server configuration to a previous version identified by its config ID. + + ##### Permissions + Must have `manage_system` permission. + operationId: RollbackConfig + requestBody: + description: The config ID to rollback to + required: true + content: + application/json: + schema: + type: object + properties: + config_id: + type: string + description: The ID of the configuration to rollback to + required: + - config_id + responses: + "200": + description: Configuration rollback successful + content: + application/json: + schema: + $ref: "#/components/schemas/Config" + "400": + $ref: "#/components/responses/BadRequest" + "401": + $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" /api/v4/config/patch: put: tags: diff --git a/server/channels/api4/config.go b/server/channels/api4/config.go index 02023e30f16..05c1efc1b86 100644 --- a/server/channels/api4/config.go +++ b/server/channels/api4/config.go @@ -520,6 +520,18 @@ func rollbackConfig(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.Success() w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") + if c.App.Channels().License().IsCloud() { + js, err := newCfg.ToJSONFiltered(model.ConfigAccessTagType, model.ConfigAccessTagCloudRestrictable) + if err != nil { + c.Err = model.NewAppError("rollbackConfig", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err) + return + } + if _, err := w.Write(js); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } + return + } + if err := json.NewEncoder(w).Encode(newCfg); err != nil { c.Logger.Warn("Error while writing response", mlog.Err(err)) } diff --git a/server/channels/app/config.go b/server/channels/app/config.go index 42522322d61..61f66e2f265 100644 --- a/server/channels/app/config.go +++ b/server/channels/app/config.go @@ -6,7 +6,9 @@ package app import ( "crypto/ecdsa" "crypto/rand" + "database/sql" "encoding/json" + stderrors "errors" "net/http" "net/url" "reflect" @@ -256,7 +258,10 @@ func (a *App) ListConfigurations(limit int, includeDiffs string) ([]*model.Confi func (a *App) RollbackConfig(id string) (*model.Config, *model.Config, *model.AppError) { historicalCfg, err := a.Srv().platform.GetConfigByID(id) if err != nil { - return nil, nil, model.NewAppError("RollbackConfig", "api.config.rollback_config.not_found.app_error", nil, "", http.StatusNotFound).Wrap(err) + if stderrors.Is(err, sql.ErrNoRows) { + return nil, nil, model.NewAppError("RollbackConfig", "api.config.rollback_config.not_found.app_error", nil, "", http.StatusNotFound).Wrap(err) + } + return nil, nil, model.NewAppError("RollbackConfig", "api.config.rollback_config.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } return a.SaveConfig(historicalCfg, true) } diff --git a/server/cmd/mmctl/commands/config.go b/server/cmd/mmctl/commands/config.go index 9ba219da51e..f9a851cbd56 100644 --- a/server/cmd/mmctl/commands/config.go +++ b/server/cmd/mmctl/commands/config.go @@ -658,7 +658,11 @@ func configListCmdF(c client.Client, cmd *cobra.Command, args []string) error { } if len(items) == 0 { - printer.Print("No configurations found. This feature requires a database-backed config store.") + if printer.GetFormat() == printer.FormatJSON { + printer.Print([]any{}) + } else { + printer.Print("No configurations found.") + } return nil } diff --git a/server/config/database.go b/server/config/database.go index ca913b22cff..a1a2c7d9058 100644 --- a/server/config/database.go +++ b/server/config/database.go @@ -366,27 +366,42 @@ func (ds *DatabaseStore) listConfigurations(limit int, includeDiffs string) ([]* LIMIT $1 ` + // When diffs are enabled, fetch one extra predecessor row so the oldest + // visible item can be diffed against its predecessor. + queryLimit := limit + if includeDiffs != "" { + queryLimit = limit + 1 + } + var rows []configListRow - if err := ds.db.Select(&rows, query, limit); err != nil { + if err := ds.db.Select(&rows, query, queryLimit); err != nil { return nil, errors.Wrap(err, "failed to list configurations") } - items := make([]*model.ConfigListItem, len(rows)) + // Build the full set for diffing, but only return up to limit items + allItems := make([]*model.ConfigListItem, len(rows)) for i, row := range rows { - items[i] = &model.ConfigListItem{ + allItems[i] = &model.ConfigListItem{ Id: row.Id, CreateAt: row.CreateAt, Active: row.Active, } } - if includeDiffs == "" || len(items) <= 1 { + // Items to return (capped at limit) + items := allItems + if len(items) > limit { + items = allItems[:limit] + } + + if includeDiffs == "" || len(allItems) <= 1 { return items, nil } - // Fetch config values and compute diffs between consecutive entries - ids := make([]string, len(items)) - for i, item := range items { + // Fetch config values for all rows (including the extra predecessor) + // and compute diffs between consecutive entries. + ids := make([]string, len(allItems)) + for i, item := range allItems { ids[i] = item.Id } @@ -409,9 +424,10 @@ func (ds *DatabaseStore) listConfigurations(limit int, includeDiffs string) ([]* configs[id] = cfg } - // Sort chronologically (oldest first) for diffing - sorted := make([]*model.ConfigListItem, len(items)) - copy(sorted, items) + // Sort chronologically (oldest first) for diffing, using allItems + // so the extra predecessor is included in the diff computation. + sorted := make([]*model.ConfigListItem, len(allItems)) + copy(sorted, allItems) sort.Slice(sorted, func(i, j int) bool { return sorted[i].CreateAt < sorted[j].CreateAt }) From a3fefdb476096bada3af33f90252cb372ba67ad6 Mon Sep 17 00:00:00 2001 From: Wayne Wollesen Date: Sun, 22 Mar 2026 09:36:09 -0700 Subject: [PATCH 5/8] Address code review nits for config list/rollback endpoints - Add cloud filtering for listConfigurations handler to strip cloud-restrictable fields from config diffs - Add audit metadata (config_id) to rollback audit record for traceability - Define ConfigChange and ConfigListItem named schemas in OpenAPI definitions - Reference ConfigListItem schema in /config/list response - Add 500 response to /config/list and 404 response to /config/rollback - Add unit test for --no-delta taking precedence over --detailed Co-Authored-By: Claude Opus 4.6 (1M context) --- api/v4/source/definitions.yaml | 30 ++++++++++++++ api/v4/source/system.yaml | 6 ++- server/channels/api4/config.go | 6 +++ server/cmd/mmctl/commands/config_test.go | 27 ++++++++++++ server/public/model/config.go | 52 ++++++++++++++++++++++++ 5 files changed, 120 insertions(+), 1 deletion(-) diff --git a/api/v4/source/definitions.yaml b/api/v4/source/definitions.yaml index aca580926fa..16c196a4204 100644 --- a/api/v4/source/definitions.yaml +++ b/api/v4/source/definitions.yaml @@ -1403,6 +1403,36 @@ components: description: Map of all available LDAP attributes additionalProperties: type: string + ConfigChange: + type: object + description: A single setting change between two configuration versions. + properties: + path: + type: string + description: The dot-separated path of the changed setting. + old_value: + description: The previous value of the setting. Only present when detailed diffs are requested. + new_value: + description: The new value of the setting. Only present when detailed diffs are requested. + ConfigListItem: + type: object + description: Metadata about a stored configuration entry. + properties: + id: + type: string + description: The unique identifier of the configuration. + create_at: + type: integer + format: int64 + description: The time in milliseconds the configuration was created. + active: + type: boolean + description: Whether this is the currently active configuration. + changes: + type: array + description: The list of setting changes from the previous configuration. + items: + $ref: "#/components/schemas/ConfigChange" Config: type: object properties: diff --git a/api/v4/source/system.yaml b/api/v4/source/system.yaml index 7c10e77998c..3eef60c9e10 100644 --- a/api/v4/source/system.yaml +++ b/api/v4/source/system.yaml @@ -521,13 +521,15 @@ schema: type: array items: - type: object + $ref: "#/components/schemas/ConfigListItem" "400": $ref: "#/components/responses/BadRequest" "401": $ref: "#/components/responses/Unauthorized" "403": $ref: "#/components/responses/Forbidden" + "500": + $ref: "#/components/responses/InternalServerError" /api/v4/config/rollback: post: tags: @@ -565,6 +567,8 @@ $ref: "#/components/responses/Unauthorized" "403": $ref: "#/components/responses/Forbidden" + "404": + $ref: "#/components/responses/NotFound" /api/v4/config/patch: put: tags: diff --git a/server/channels/api4/config.go b/server/channels/api4/config.go index 05c1efc1b86..913a6904ab7 100644 --- a/server/channels/api4/config.go +++ b/server/channels/api4/config.go @@ -486,6 +486,10 @@ func listConfigurations(c *Context, w http.ResponseWriter, r *http.Request) { return } + if c.App.Channels().License().IsCloud() { + model.FilterCloudRestrictedChanges(items) + } + auditRec.Success() w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") if err := json.NewEncoder(w).Encode(items); err != nil { @@ -510,6 +514,8 @@ func rollbackConfig(c *Context, w http.ResponseWriter, r *http.Request) { return } + auditRec.AddMeta("config_id", body.ConfigID) + _, newCfg, appErr := c.App.RollbackConfig(body.ConfigID) if appErr != nil { c.Err = appErr diff --git a/server/cmd/mmctl/commands/config_test.go b/server/cmd/mmctl/commands/config_test.go index c4e704ead51..35cf4c9a273 100644 --- a/server/cmd/mmctl/commands/config_test.go +++ b/server/cmd/mmctl/commands/config_test.go @@ -1139,6 +1139,33 @@ func (s *MmctlUnitTestSuite) TestConfigListCmd() { s.Require().Equal("https://mm.example.com", item.Changes[0].NewValue) }) + s.Run("No-delta takes precedence over detailed", func() { + printer.Clean() + items := []*model.ConfigListItem{ + {Id: "abc123def456ghi789jkl0mn", CreateAt: 1700000000000, Active: true}, + {Id: "xyz789abc012def345ghi6mn", CreateAt: 1699999000000, Active: false}, + } + + s.client. + EXPECT(). + ListConfigurations(context.TODO(), 5, ""). + Return(items, &model.Response{}, nil). + Times(1) + + cmd := newListCmd() + _ = cmd.Flags().Set("detailed", "true") + _ = cmd.Flags().Set("no-delta", "true") + + err := configListCmdF(s.client, cmd, []string{}) + s.Require().Nil(err) + s.Require().Len(printer.GetLines(), 2) + // Verify no changes are present on either item + for _, line := range printer.GetLines() { + item := line.(*model.ConfigListItem) + s.Require().Empty(item.Changes) + } + }) + s.Run("No-delta mode skips diffs", func() { printer.Clean() items := []*model.ConfigListItem{ diff --git a/server/public/model/config.go b/server/public/model/config.go index 0bb04236c1b..8c267f15b2e 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -5163,6 +5163,58 @@ type ConfigListItem struct { Changes []ConfigChange `json:"changes,omitempty"` } +// collectTaggedPaths walks a struct type and returns all dot-separated field +// paths that carry the given tag value in the specified tag key. +func collectTaggedPaths(t reflect.Type, tagKey, tagValue, prefix string) map[string]bool { + paths := map[string]bool{} + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + fieldPath := field.Name + if prefix != "" { + fieldPath = prefix + "." + field.Name + } + + tags := strings.Split(field.Tag.Get(tagKey), ",") + if isTagPresent(tagValue, tags) { + paths[fieldPath] = true + continue + } + + ft := field.Type + if ft.Kind() == reflect.Ptr { + ft = ft.Elem() + } + if ft.Kind() == reflect.Struct { + for p := range collectTaggedPaths(ft, tagKey, tagValue, fieldPath) { + paths[p] = true + } + } + } + return paths +} + +// CloudRestrictedPaths returns the set of config field paths tagged as cloud_restrictable. +func CloudRestrictedPaths() map[string]bool { + return collectTaggedPaths(reflect.TypeOf(Config{}), ConfigAccessTagType, ConfigAccessTagCloudRestrictable, "") +} + +// FilterCloudRestrictedChanges removes changes whose paths match cloud-restrictable config fields. +func FilterCloudRestrictedChanges(items []*ConfigListItem) { + restricted := CloudRestrictedPaths() + for _, item := range items { + if len(item.Changes) == 0 { + continue + } + filtered := make([]ConfigChange, 0, len(item.Changes)) + for _, ch := range item.Changes { + if !restricted[ch.Path] { + filtered = append(filtered, ch) + } + } + item.Changes = filtered + } +} + // FilterConfig returns a map[string]any representation of the configuration. // Also, the function can filter the configuration by the options passed // in the argument. The options are used to remove the default values, the masked From 4563d7fc3f1b3b42347cada734d3bdf5e20e7b5c Mon Sep 17 00:00:00 2001 From: Wayne Wollesen Date: Sun, 22 Mar 2026 10:16:29 -0700 Subject: [PATCH 6/8] Use reflect.TypeFor instead of reflect.TypeOf to satisfy modernize linter Co-Authored-By: Claude Opus 4.6 (1M context) --- server/public/model/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/public/model/config.go b/server/public/model/config.go index 8c267f15b2e..27a299c2554 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -5195,7 +5195,7 @@ func collectTaggedPaths(t reflect.Type, tagKey, tagValue, prefix string) map[str // CloudRestrictedPaths returns the set of config field paths tagged as cloud_restrictable. func CloudRestrictedPaths() map[string]bool { - return collectTaggedPaths(reflect.TypeOf(Config{}), ConfigAccessTagType, ConfigAccessTagCloudRestrictable, "") + return collectTaggedPaths(reflect.TypeFor[Config](), ConfigAccessTagType, ConfigAccessTagCloudRestrictable, "") } // FilterCloudRestrictedChanges removes changes whose paths match cloud-restrictable config fields. From dfe73b14c6295dddbc11916468e0acf0187fa802 Mon Sep 17 00:00:00 2001 From: Wayne Wollesen Date: Sun, 22 Mar 2026 10:44:27 -0700 Subject: [PATCH 7/8] Filter cloud-restricted descendants by ancestor path prefix, not exact match The cloud filtering for config diffs now matches descendant and indexed paths (e.g. SqlSettings.ReplicaLagSettings[0].DataSource) under tagged ancestor fields, not just exact path equality. Co-Authored-By: Claude Opus 4.6 (1M context) --- server/public/model/config.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/server/public/model/config.go b/server/public/model/config.go index 27a299c2554..3b7f653abaf 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -5198,7 +5198,23 @@ func CloudRestrictedPaths() map[string]bool { return collectTaggedPaths(reflect.TypeFor[Config](), ConfigAccessTagType, ConfigAccessTagCloudRestrictable, "") } -// FilterCloudRestrictedChanges removes changes whose paths match cloud-restrictable config fields. +// isCloudRestricted checks whether a change path matches or is a descendant +// of any cloud-restrictable config field path (e.g. "SqlSettings.ReplicaLagSettings" +// also matches "SqlSettings.ReplicaLagSettings[0].DataSource"). +func isCloudRestricted(path string, restricted map[string]bool) bool { + if restricted[path] { + return true + } + for rp := range restricted { + if strings.HasPrefix(path, rp+".") || strings.HasPrefix(path, rp+"[") { + return true + } + } + return false +} + +// FilterCloudRestrictedChanges removes changes whose paths match or descend from +// cloud-restrictable config fields. func FilterCloudRestrictedChanges(items []*ConfigListItem) { restricted := CloudRestrictedPaths() for _, item := range items { @@ -5207,7 +5223,7 @@ func FilterCloudRestrictedChanges(items []*ConfigListItem) { } filtered := make([]ConfigChange, 0, len(item.Changes)) for _, ch := range item.Changes { - if !restricted[ch.Path] { + if !isCloudRestricted(ch.Path, restricted) { filtered = append(filtered, ch) } } From 9a302b698fbf30eef3afbe23500ed14dbf3ac5a4 Mon Sep 17 00:00:00 2001 From: Wayne Wollesen Date: Sun, 22 Mar 2026 10:50:37 -0700 Subject: [PATCH 8/8] Add doc comments to exported functions for docstring coverage Co-Authored-By: Claude Opus 4.6 (1M context) --- server/channels/app/config.go | 2 ++ server/channels/app/platform/config.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/server/channels/app/config.go b/server/channels/app/config.go index 61f66e2f265..90cca0ae74b 100644 --- a/server/channels/app/config.go +++ b/server/channels/app/config.go @@ -247,6 +247,7 @@ func (a *App) SaveConfig(newCfg *model.Config, sendConfigChangeClusterMessage bo return a.Srv().platform.SaveConfig(newCfg, sendConfigChangeClusterMessage) } +// ListConfigurations returns metadata for stored configuration entries with optional diffs. func (a *App) ListConfigurations(limit int, includeDiffs string) ([]*model.ConfigListItem, *model.AppError) { items, err := a.Srv().platform.ListConfigurations(limit, includeDiffs) if err != nil { @@ -255,6 +256,7 @@ func (a *App) ListConfigurations(limit int, includeDiffs string) ([]*model.Confi return items, nil } +// RollbackConfig restores a historical configuration identified by its ID. func (a *App) RollbackConfig(id string) (*model.Config, *model.Config, *model.AppError) { historicalCfg, err := a.Srv().platform.GetConfigByID(id) if err != nil { diff --git a/server/channels/app/platform/config.go b/server/channels/app/platform/config.go index 0c3a5c9131c..a6fdd526d7f 100644 --- a/server/channels/app/platform/config.go +++ b/server/channels/app/platform/config.go @@ -151,10 +151,12 @@ func (ps *PlatformService) CleanUpConfig() error { return ps.configStore.CleanUp() } +// ListConfigurations delegates to the config store to retrieve configuration history. func (ps *PlatformService) ListConfigurations(limit int, includeDiffs string) ([]*model.ConfigListItem, error) { return ps.configStore.ListConfigurations(limit, includeDiffs) } +// GetConfigByID delegates to the config store to retrieve a configuration by its ID. func (ps *PlatformService) GetConfigByID(id string) (*model.Config, error) { return ps.configStore.GetConfigByID(id) }