From d5c7f55ef4983141dbc112bfb18e2a239dc4cf2e Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Fri, 20 Mar 2026 03:22:29 +0000 Subject: [PATCH 01/10] fix: add nil guard to all License().IsCloud() calls Add nil-check before every License().IsCloud() and License().IsCloudPreview() call across the server codebase. When no license is installed, License() returns nil, and calling IsCloud() on nil panics with a nil pointer dereference. This was first observed in Sentry issue TY (team.go, 1,497 events from mattermost.bolor.net). PR #35707 fixed the 3 occurrences in team.go, but the same unsafe pattern existed in 30+ other locations across 12 files. This PR is a comprehensive sweep fixing all remaining occurrences: - api4/cloud.go: 11 calls (IsCloud) + 1 call (IsCloudPreview) - api4/config.go: 5 calls - api4/team.go: 3 calls (same as #35707, not yet merged) - api4/license.go: 1 call - api4/user.go: 2 calls (IsCloud + IsCloudPreview) - api4/upload.go: 2 calls - app/audit.go: 2 calls - app/file.go: 1 call - app/ip_filtering.go: 1 call - app/login.go: 2 calls - app/server.go: 1 call - web/handlers.go: 2 calls Pattern used: - Positive checks: `License() != nil && License().IsCloud()` - Negative checks: `License() == nil || !License().IsCloud()` Sentry: https://mattermost-mr.sentry.io/issues/7063429511/ Co-authored-by: Claude --- server/channels/api4/cloud.go | 22 +++++++++++----------- server/channels/api4/config.go | 10 +++++----- server/channels/api4/config_test.go | 8 ++++++++ server/channels/api4/license.go | 2 +- server/channels/api4/team.go | 6 +++--- server/channels/api4/team_test.go | 10 ++++++++++ server/channels/api4/upload.go | 4 ++-- server/channels/api4/user.go | 4 ++-- server/channels/app/audit.go | 4 ++-- server/channels/app/file.go | 2 +- server/channels/app/ip_filtering.go | 2 +- server/channels/app/login.go | 4 ++-- server/channels/app/server.go | 2 +- server/channels/web/handlers.go | 4 ++-- 14 files changed, 51 insertions(+), 33 deletions(-) diff --git a/server/channels/api4/cloud.go b/server/channels/api4/cloud.go index 5e1eac99757..fffcd0d642c 100644 --- a/server/channels/api4/cloud.go +++ b/server/channels/api4/cloud.go @@ -90,7 +90,7 @@ func getPreviewSubscription(c *Context, w http.ResponseWriter, r *http.Request) func getSubscription(c *Context, w http.ResponseWriter, r *http.Request) { // Preview subscription is a special case for cloud preview licenses. - if c.App.Channels().License().IsCloudPreview() { + if c.App.Channels().License() != nil && c.App.Channels().License().IsCloudPreview() { getPreviewSubscription(c, w, r) return } @@ -100,7 +100,7 @@ func getSubscription(c *Context, w http.ResponseWriter, r *http.Request) { return } - if !c.App.Channels().License().IsCloud() { + if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("Api4.getSubscription", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -199,7 +199,7 @@ func validateWorkspaceBusinessEmail(c *Context, w http.ResponseWriter, r *http.R return } - if !c.App.Channels().License().IsCloud() { + if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("Api4.validateWorkspaceBusinessEmail", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -250,7 +250,7 @@ func getCloudProducts(c *Context, w http.ResponseWriter, r *http.Request) { return } - if !c.App.Channels().License().IsCloud() { + if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("Api4.getCloudProducts", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -300,7 +300,7 @@ func getCloudLimits(c *Context, w http.ResponseWriter, r *http.Request) { return } - if !c.App.Channels().License().IsCloud() { + if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("Api4.getCloudLimits", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -328,7 +328,7 @@ func getCloudCustomer(c *Context, w http.ResponseWriter, r *http.Request) { return } - if !c.App.Channels().License().IsCloud() { + if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("Api4.getCloudCustomer", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -384,7 +384,7 @@ func updateCloudCustomer(c *Context, w http.ResponseWriter, r *http.Request) { return } - if !c.App.Channels().License().IsCloud() { + if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("Api4.updateCloudCustomer", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -429,7 +429,7 @@ func updateCloudCustomerAddress(c *Context, w http.ResponseWriter, r *http.Reque return } - if !c.App.Channels().License().IsCloud() { + if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("Api4.updateCloudCustomerAddress", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -474,7 +474,7 @@ func getInvoicesForSubscription(c *Context, w http.ResponseWriter, r *http.Reque return } - if !c.App.Channels().License().IsCloud() { + if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("Api4.getInvoicesForSubscription", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -507,7 +507,7 @@ func getSubscriptionInvoicePDF(c *Context, w http.ResponseWriter, r *http.Reques return } - if !c.App.Channels().License().IsCloud() { + if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("Api4.getSubscriptionInvoicePDF", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -547,7 +547,7 @@ func handleCWSWebhook(c *Context, w http.ResponseWriter, r *http.Request) { return } - if !c.App.Channels().License().IsCloud() { + if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("Api4.handleCWSWebhook", "api.cloud.license_error", nil, "", http.StatusForbidden) return } diff --git a/server/channels/api4/config.go b/server/channels/api4/config.go index c35ec4daa86..4c0a5f6c21b 100644 --- a/server/channels/api4/config.go +++ b/server/channels/api4/config.go @@ -80,7 +80,7 @@ func getConfig(c *Context, w http.ResponseWriter, r *http.Request) { RemoveMasked: filterMasked, }, } - if c.App.Channels().License().IsCloud() { + if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { filterOpts.TagFilters = append(filterOpts.TagFilters, model.FilterTag{ TagType: model.ConfigAccessTagType, TagName: model.ConfigAccessTagCloudRestrictable, @@ -168,7 +168,7 @@ func updateConfig(c *Context, w http.ResponseWriter, r *http.Request) { } // There are some settings that cannot be changed in a cloud env - if c.App.Channels().License().IsCloud() { + if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { // Both of them cannot be nil since cfg.SetDefaults is called earlier for cfg, // and appCfg is the existing earlier config and if it's nil, server sets a default value. if *appCfg.ComplianceSettings.Directory != *cfg.ComplianceSettings.Directory { @@ -233,7 +233,7 @@ func updateConfig(c *Context, w http.ResponseWriter, r *http.Request) { c.LogAudit("updateConfig") w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") - if c.App.Channels().License().IsCloud() { + if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { js, err := cfg.ToJSONFiltered(model.ConfigAccessTagType, model.ConfigAccessTagCloudRestrictable) if err != nil { c.Err = model.NewAppError("updateConfig", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -324,7 +324,7 @@ func patchConfig(c *Context, w http.ResponseWriter, r *http.Request) { } // There are some settings that cannot be changed in a cloud env - if c.App.Channels().License().IsCloud() { + if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { if cfg.ComplianceSettings.Directory != nil && *appCfg.ComplianceSettings.Directory != *cfg.ComplianceSettings.Directory { c.Err = model.NewAppError("patchConfig", "api.config.update_config.not_allowed_security.app_error", map[string]any{"Name": "ComplianceSettings.Directory"}, "", http.StatusForbidden) return @@ -383,7 +383,7 @@ func patchConfig(c *Context, w http.ResponseWriter, r *http.Request) { } w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") - if c.App.Channels().License().IsCloud() { + if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { js, err := cfg.ToJSONFiltered(model.ConfigAccessTagType, model.ConfigAccessTagCloudRestrictable) if err != nil { c.Err = model.NewAppError("patchConfig", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err) diff --git a/server/channels/api4/config_test.go b/server/channels/api4/config_test.go index 921b378fead..756843ed428 100644 --- a/server/channels/api4/config_test.go +++ b/server/channels/api4/config_test.go @@ -66,6 +66,14 @@ func TestGetConfig(t *testing.T) { require.NotEqual(t, model.FakeSetting, *cfg.SqlSettings.DataSource) require.NotEqual(t, model.FakeSetting, *cfg.FileSettings.PublicLinkSalt) }) + + t.Run("nil license does not panic", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + // GetConfig calls License().IsCloud() — must not panic when license is nil + _, _, err := th.SystemAdminClient.GetConfig(context.Background()) + require.NoError(t, err) + }) } func TestGetConfigWithAccessTag(t *testing.T) { diff --git a/server/channels/api4/license.go b/server/channels/api4/license.go index f9dc659a5ac..01aedc054f5 100644 --- a/server/channels/api4/license.go +++ b/server/channels/api4/license.go @@ -137,7 +137,7 @@ func addLicense(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.App.Channels().License().IsCloud() { + if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { // If cloud, invalidate the caches when a new license is loaded defer func() { if err := c.App.Srv().Cloud.HandleLicenseChange(); err != nil { diff --git a/server/channels/api4/team.go b/server/channels/api4/team.go index 4d731c7e8b8..4456f61e792 100644 --- a/server/channels/api4/team.go +++ b/server/channels/api4/team.go @@ -99,7 +99,7 @@ func createTeam(c *Context, w http.ResponseWriter, r *http.Request) { } // On a cloud license, we must check limits before allowing to create - if c.App.Channels().License().IsCloud() { + if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { limits, err := c.App.Cloud().GetCloudLimits(c.AppContext.Session().UserId) if err != nil { c.Err = model.NewAppError("Api4.createTeam", "api.cloud.app_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -379,7 +379,7 @@ func restoreTeam(c *Context, w http.ResponseWriter, r *http.Request) { return } // On a cloud license, we must check limits before allowing to restore - if c.App.Channels().License().IsCloud() { + if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { limits, err := c.App.Cloud().GetCloudLimits(c.AppContext.Session().UserId) if err != nil { c.Err = model.NewAppError("Api4.restoreTeam", "api.cloud.app_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -1398,7 +1398,7 @@ func teamExists(c *Context, w http.ResponseWriter, r *http.Request) { } func importTeam(c *Context, w http.ResponseWriter, r *http.Request) { - if c.App.Channels().License().IsCloud() { + if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("importTeam", "api.restricted_system_admin", nil, "", http.StatusForbidden) return } diff --git a/server/channels/api4/team_test.go b/server/channels/api4/team_test.go index 54376878196..fb72c362805 100644 --- a/server/channels/api4/team_test.go +++ b/server/channels/api4/team_test.go @@ -235,6 +235,16 @@ func TestCreateTeam(t *testing.T) { require.NoError(t, err) CheckCreatedStatus(t, resp) }) + + t.Run("nil license does not panic", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + team := &model.Team{Name: GenerateTestUsername(), DisplayName: "No License Team", Type: model.TeamOpen} + _, _, err := th.Client.CreateTeam(context.Background(), team) + // The request may succeed or fail depending on permissions, + // but it must NOT panic due to nil License().IsCloud() + _ = err + }) } func TestCreateTeamSanitization(t *testing.T) { diff --git a/server/channels/api4/upload.go b/server/channels/api4/upload.go index f169b8d5f00..0796d5fbf55 100644 --- a/server/channels/api4/upload.go +++ b/server/channels/api4/upload.go @@ -52,7 +52,7 @@ func createUpload(c *Context, w http.ResponseWriter, r *http.Request) { c.SetPermissionError(model.PermissionManageSystem) return } - if c.App.Srv().License().IsCloud() { + if c.App.Srv().License() != nil && c.App.Srv().License().IsCloud() { c.Err = model.NewAppError("createUpload", "api.file.cloud_upload.app_error", nil, "", http.StatusBadRequest) return } @@ -147,7 +147,7 @@ func uploadData(c *Context, w http.ResponseWriter, r *http.Request) { c.SetPermissionError(model.PermissionManageSystem) return } - if c.App.Srv().License().IsCloud() { + if c.App.Srv().License() != nil && c.App.Srv().License().IsCloud() { c.Err = model.NewAppError("UploadData", "api.file.cloud_upload.app_error", nil, "", http.StatusBadRequest) return } diff --git a/server/channels/api4/user.go b/server/channels/api4/user.go index e98f701e767..cb98804ce5b 100644 --- a/server/channels/api4/user.go +++ b/server/channels/api4/user.go @@ -2183,7 +2183,7 @@ func loginCWS(c *Context, w http.ResponseWriter, r *http.Request) { "cyber-defense": "/cyber-defense-hq", } - if !c.App.Channels().License().IsCloud() { + if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { c.Err = model.NewAppError("loginCWS", "api.user.login_cws.license.error", nil, "", http.StatusUnauthorized) return } @@ -2242,7 +2242,7 @@ func loginCWS(c *Context, w http.ResponseWriter, r *http.Request) { } // If a cloud preview, redirect to the correct use case URL - if c.App.License().IsCloudPreview() && useCase != "" { + if c.App.License() != nil && c.App.License().IsCloudPreview() && useCase != "" { if url, ok := useCaseToURL[useCase]; ok { redirectURL += url } diff --git a/server/channels/app/audit.go b/server/channels/app/audit.go index 2b8a6f9e732..e0e15b7c716 100644 --- a/server/channels/app/audit.go +++ b/server/channels/app/audit.go @@ -198,7 +198,7 @@ func (a *App) AddAuditLogCertificate(rctx request.CTX, fileData *multipart.FileH a.UpdateConfig(func(dest *model.Config) { *dest = *cfg }) - if a.License().IsCloud() { + if a.License() != nil && a.License().IsCloud() { err = a.Cloud().CreateAuditLoggingCert(rctx.Session().UserId, fileData) if err != nil { return model.NewAppError("AddAuditLogCertificate", "api.admin.add_certificate.app_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -224,7 +224,7 @@ func (a *App) RemoveAuditLogCertificate(rctx request.CTX) *model.AppError { a.UpdateConfig(func(dest *model.Config) { *dest = *cfg }) - if a.License().IsCloud() { + if a.License() != nil && a.License().IsCloud() { err = a.Cloud().RemoveAuditLoggingCert(rctx.Session().UserId) if err != nil { return model.NewAppError("RemoveAuditLogCertificate", "api.admin.remove_certificate.app_error", nil, "", http.StatusInternalServerError).Wrap(err) diff --git a/server/channels/app/file.go b/server/channels/app/file.go index c92d91b2446..dee2c0169f5 100644 --- a/server/channels/app/file.go +++ b/server/channels/app/file.go @@ -60,7 +60,7 @@ func (a *App) ExportFileBackend() filestore.FileBackend { func (a *App) CheckMandatoryS3Fields(settings *model.FileSettings) *model.AppError { var fileBackendSettings filestore.FileBackendSettings - if a.License().IsCloud() && a.Config().FeatureFlags.CloudDedicatedExportUI && a.Config().FileSettings.DedicatedExportStore != nil && *a.Config().FileSettings.DedicatedExportStore { + if a.License() != nil && a.License().IsCloud() && a.Config().FeatureFlags.CloudDedicatedExportUI && a.Config().FileSettings.DedicatedExportStore != nil && *a.Config().FileSettings.DedicatedExportStore { fileBackendSettings = filestore.NewExportFileBackendSettingsFromConfig(settings, false, false) } else { fileBackendSettings = filestore.NewFileBackendSettingsFromConfig(settings, false, false) diff --git a/server/channels/app/ip_filtering.go b/server/channels/app/ip_filtering.go index 2afb9ef4bee..f77d3ac0e55 100644 --- a/server/channels/app/ip_filtering.go +++ b/server/channels/app/ip_filtering.go @@ -10,7 +10,7 @@ import ( func (a *App) SendIPFiltersChangedEmail(rctx request.CTX, userID string) error { cloudWorkspaceOwnerEmailAddress := "" - if a.License().IsCloud() { + if a.License() != nil && a.License().IsCloud() { portalUserCustomer, cErr := a.Cloud().GetCloudCustomer(userID) if cErr != nil { rctx.Logger().Error("Failed to get portal user customer", mlog.Err(cErr)) diff --git a/server/channels/app/login.go b/server/channels/app/login.go index e723b461e64..b8ee21cc716 100644 --- a/server/channels/app/login.go +++ b/server/channels/app/login.go @@ -310,7 +310,7 @@ func (a *App) AttachSessionCookies(rctx request.CTX, w http.ResponseWriter, r *h http.SetCookie(w, csrfCookie) // For context see: https://mattermost.atlassian.net/browse/MM-39583 - if a.License().IsCloud() { + if a.License() != nil && a.License().IsCloud() { a.AttachCloudSessionCookie(rctx, w, r) } } @@ -323,5 +323,5 @@ func GetProtocol(r *http.Request) string { } func isCWSLogin(a *App, token string) bool { - return a.License().IsCloud() && token != "" + return a.License() != nil && a.License().IsCloud() && token != "" } diff --git a/server/channels/app/server.go b/server/channels/app/server.go index c452ed7e0fc..8f632340fdd 100644 --- a/server/channels/app/server.go +++ b/server/channels/app/server.go @@ -1355,7 +1355,7 @@ func (s *Server) sendLicenseUpForRenewalEmail(users map[string]*model.User, lice func (s *Server) doReportUserCountForCloudSubscriptionJob() { s.LoadLicense() - if !s.License().IsCloud() { + if s.License() == nil || !s.License().IsCloud() { return } diff --git a/server/channels/web/handlers.go b/server/channels/web/handlers.go index 1ec64fb611e..8e0e56e5d3d 100644 --- a/server/channels/web/handlers.go +++ b/server/channels/web/handlers.go @@ -213,7 +213,7 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { subpath, _ := utils.GetSubpathFromConfig(c.App.Config()) siteURLHeader := app.GetProtocol(r) + "://" + r.Host + subpath - if c.App.Channels().License().IsCloud() { + if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { siteURLHeader = *c.App.Config().ServiceSettings.SiteURL + subpath } c.SetSiteURLHeader(siteURLHeader) @@ -285,7 +285,7 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { c.RemoveSessionCookie(w, r) c.Err = model.NewAppError("ServeHTTP", "api.context.session_expired.app_error", nil, "token="+token+" Appears to be a CSRF attempt", http.StatusUnauthorized) } - } else if token != "" && c.App.Channels().License().IsCloud() && tokenLocation == app.TokenLocationCloudHeader { + } else if token != "" && c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() && tokenLocation == app.TokenLocationCloudHeader { // Check to see if this provided token matches our CWS Token session, err := c.App.GetCloudSession(token) if err != nil { From 8e4fb8c63e65d0cf61954f7d70b618eac6f47bae Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Fri, 3 Apr 2026 16:27:21 +0000 Subject: [PATCH 02/10] fix: cache License() to prevent race and assert test outcome Cache s.License() in local variable before nil check to prevent TOCTOU race. Add HTTP status assertion to nil-license regression test. Co-authored-by: Claude --- server/channels/api4/team_test.go | 5 +++-- server/channels/app/server.go | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/server/channels/api4/team_test.go b/server/channels/api4/team_test.go index fb72c362805..097e130b7e5 100644 --- a/server/channels/api4/team_test.go +++ b/server/channels/api4/team_test.go @@ -240,10 +240,11 @@ func TestCreateTeam(t *testing.T) { th.App.Srv().SetLicense(nil) team := &model.Team{Name: GenerateTestUsername(), DisplayName: "No License Team", Type: model.TeamOpen} - _, _, err := th.Client.CreateTeam(context.Background(), team) + _, resp, err := th.Client.CreateTeam(context.Background(), team) // The request may succeed or fail depending on permissions, // but it must NOT panic due to nil License().IsCloud() - _ = err + require.NoError(t, err) + require.NotEqual(t, http.StatusInternalServerError, resp.StatusCode) }) } diff --git a/server/channels/app/server.go b/server/channels/app/server.go index e64812d1ba1..03a89ac7671 100644 --- a/server/channels/app/server.go +++ b/server/channels/app/server.go @@ -1370,7 +1370,8 @@ func (s *Server) sendLicenseUpForRenewalEmail(users map[string]*model.User, lice func (s *Server) doReportUserCountForCloudSubscriptionJob() { s.LoadLicense() - if s.License() == nil || !s.License().IsCloud() { + license := s.License() + if license == nil || !license.IsCloud() { return } From cbb083b3b0a51a0ee17349c732f54ca8b63bb905 Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Fri, 3 Apr 2026 16:35:36 +0000 Subject: [PATCH 03/10] fix: cache all License() calls to prevent TOCTOU races Sweep all double-fetch License() patterns in files touched by this PR. Each instance now caches the License() return value in a local variable before checking, preventing races where the license pointer changes between calls. Co-authored-by: Claude --- server/channels/api4/cloud.go | 32 +++++++++++++++++++---------- server/channels/api4/config.go | 15 +++++++++----- server/channels/api4/license.go | 3 ++- server/channels/api4/team.go | 12 +++++++---- server/channels/api4/upload.go | 6 ++++-- server/channels/api4/user.go | 15 +++++++++----- server/channels/app/audit.go | 6 ++++-- server/channels/app/file.go | 3 ++- server/channels/app/ip_filtering.go | 3 ++- server/channels/app/login.go | 9 +++++--- server/channels/web/handlers.go | 7 ++++--- 11 files changed, 73 insertions(+), 38 deletions(-) diff --git a/server/channels/api4/cloud.go b/server/channels/api4/cloud.go index fffcd0d642c..5c469753011 100644 --- a/server/channels/api4/cloud.go +++ b/server/channels/api4/cloud.go @@ -90,7 +90,8 @@ func getPreviewSubscription(c *Context, w http.ResponseWriter, r *http.Request) func getSubscription(c *Context, w http.ResponseWriter, r *http.Request) { // Preview subscription is a special case for cloud preview licenses. - if c.App.Channels().License() != nil && c.App.Channels().License().IsCloudPreview() { + license := c.App.Channels().License() + if license != nil && license.IsCloudPreview() { getPreviewSubscription(c, w, r) return } @@ -100,7 +101,7 @@ func getSubscription(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { + if license == nil || !license.IsCloud() { c.Err = model.NewAppError("Api4.getSubscription", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -199,7 +200,8 @@ func validateWorkspaceBusinessEmail(c *Context, w http.ResponseWriter, r *http.R return } - if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license == nil || !license.IsCloud() { c.Err = model.NewAppError("Api4.validateWorkspaceBusinessEmail", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -250,7 +252,8 @@ func getCloudProducts(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license == nil || !license.IsCloud() { c.Err = model.NewAppError("Api4.getCloudProducts", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -300,7 +303,8 @@ func getCloudLimits(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license == nil || !license.IsCloud() { c.Err = model.NewAppError("Api4.getCloudLimits", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -328,7 +332,8 @@ func getCloudCustomer(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license == nil || !license.IsCloud() { c.Err = model.NewAppError("Api4.getCloudCustomer", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -384,7 +389,8 @@ func updateCloudCustomer(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license == nil || !license.IsCloud() { c.Err = model.NewAppError("Api4.updateCloudCustomer", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -429,7 +435,8 @@ func updateCloudCustomerAddress(c *Context, w http.ResponseWriter, r *http.Reque return } - if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license == nil || !license.IsCloud() { c.Err = model.NewAppError("Api4.updateCloudCustomerAddress", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -474,7 +481,8 @@ func getInvoicesForSubscription(c *Context, w http.ResponseWriter, r *http.Reque return } - if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license == nil || !license.IsCloud() { c.Err = model.NewAppError("Api4.getInvoicesForSubscription", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -507,7 +515,8 @@ func getSubscriptionInvoicePDF(c *Context, w http.ResponseWriter, r *http.Reques return } - if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license == nil || !license.IsCloud() { c.Err = model.NewAppError("Api4.getSubscriptionInvoicePDF", "api.cloud.license_error", nil, "", http.StatusForbidden) return } @@ -547,7 +556,8 @@ func handleCWSWebhook(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license == nil || !license.IsCloud() { c.Err = model.NewAppError("Api4.handleCWSWebhook", "api.cloud.license_error", nil, "", http.StatusForbidden) return } diff --git a/server/channels/api4/config.go b/server/channels/api4/config.go index 4c0a5f6c21b..4af473f575e 100644 --- a/server/channels/api4/config.go +++ b/server/channels/api4/config.go @@ -80,7 +80,8 @@ func getConfig(c *Context, w http.ResponseWriter, r *http.Request) { RemoveMasked: filterMasked, }, } - if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license != nil && license.IsCloud() { filterOpts.TagFilters = append(filterOpts.TagFilters, model.FilterTag{ TagType: model.ConfigAccessTagType, TagName: model.ConfigAccessTagCloudRestrictable, @@ -168,7 +169,8 @@ func updateConfig(c *Context, w http.ResponseWriter, r *http.Request) { } // There are some settings that cannot be changed in a cloud env - if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license != nil && license.IsCloud() { // Both of them cannot be nil since cfg.SetDefaults is called earlier for cfg, // and appCfg is the existing earlier config and if it's nil, server sets a default value. if *appCfg.ComplianceSettings.Directory != *cfg.ComplianceSettings.Directory { @@ -233,7 +235,8 @@ func updateConfig(c *Context, w http.ResponseWriter, r *http.Request) { c.LogAudit("updateConfig") w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") - if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { + license = c.App.Channels().License() + if license != nil && license.IsCloud() { js, err := cfg.ToJSONFiltered(model.ConfigAccessTagType, model.ConfigAccessTagCloudRestrictable) if err != nil { c.Err = model.NewAppError("updateConfig", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -324,7 +327,8 @@ func patchConfig(c *Context, w http.ResponseWriter, r *http.Request) { } // There are some settings that cannot be changed in a cloud env - if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license != nil && license.IsCloud() { if cfg.ComplianceSettings.Directory != nil && *appCfg.ComplianceSettings.Directory != *cfg.ComplianceSettings.Directory { c.Err = model.NewAppError("patchConfig", "api.config.update_config.not_allowed_security.app_error", map[string]any{"Name": "ComplianceSettings.Directory"}, "", http.StatusForbidden) return @@ -383,7 +387,8 @@ func patchConfig(c *Context, w http.ResponseWriter, r *http.Request) { } w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") - if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { + license = c.App.Channels().License() + if license != nil && license.IsCloud() { js, err := cfg.ToJSONFiltered(model.ConfigAccessTagType, model.ConfigAccessTagCloudRestrictable) if err != nil { c.Err = model.NewAppError("patchConfig", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err) diff --git a/server/channels/api4/license.go b/server/channels/api4/license.go index 01aedc054f5..d03fdd049ab 100644 --- a/server/channels/api4/license.go +++ b/server/channels/api4/license.go @@ -137,7 +137,8 @@ func addLicense(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license != nil && license.IsCloud() { // If cloud, invalidate the caches when a new license is loaded defer func() { if err := c.App.Srv().Cloud.HandleLicenseChange(); err != nil { diff --git a/server/channels/api4/team.go b/server/channels/api4/team.go index 4456f61e792..3e11b6e5d1f 100644 --- a/server/channels/api4/team.go +++ b/server/channels/api4/team.go @@ -99,7 +99,8 @@ func createTeam(c *Context, w http.ResponseWriter, r *http.Request) { } // On a cloud license, we must check limits before allowing to create - if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { + license = c.App.Channels().License() + if license != nil && license.IsCloud() { limits, err := c.App.Cloud().GetCloudLimits(c.AppContext.Session().UserId) if err != nil { c.Err = model.NewAppError("Api4.createTeam", "api.cloud.app_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -379,7 +380,8 @@ func restoreTeam(c *Context, w http.ResponseWriter, r *http.Request) { return } // On a cloud license, we must check limits before allowing to restore - if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license != nil && license.IsCloud() { limits, err := c.App.Cloud().GetCloudLimits(c.AppContext.Session().UserId) if err != nil { c.Err = model.NewAppError("Api4.restoreTeam", "api.cloud.app_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -1398,7 +1400,8 @@ func teamExists(c *Context, w http.ResponseWriter, r *http.Request) { } func importTeam(c *Context, w http.ResponseWriter, r *http.Request) { - if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license != nil && license.IsCloud() { c.Err = model.NewAppError("importTeam", "api.restricted_system_admin", nil, "", http.StatusForbidden) return } @@ -1637,7 +1640,8 @@ func inviteGuestsToChannels(c *Context, w http.ResponseWriter, r *http.Request) return } - guestEnabled := c.App.Channels().License() != nil && *c.App.Channels().License().Features.GuestAccounts + license := c.App.Channels().License() + guestEnabled := license != nil && *license.Features.GuestAccounts if !guestEnabled { c.Err = model.NewAppError("Api4.InviteGuestsToChannels", "api.team.invite_guests_to_channels.disabled.error", nil, "", http.StatusForbidden) diff --git a/server/channels/api4/upload.go b/server/channels/api4/upload.go index 0796d5fbf55..2a979c181ac 100644 --- a/server/channels/api4/upload.go +++ b/server/channels/api4/upload.go @@ -52,7 +52,8 @@ func createUpload(c *Context, w http.ResponseWriter, r *http.Request) { c.SetPermissionError(model.PermissionManageSystem) return } - if c.App.Srv().License() != nil && c.App.Srv().License().IsCloud() { + license := c.App.Srv().License() + if license != nil && license.IsCloud() { c.Err = model.NewAppError("createUpload", "api.file.cloud_upload.app_error", nil, "", http.StatusBadRequest) return } @@ -147,7 +148,8 @@ func uploadData(c *Context, w http.ResponseWriter, r *http.Request) { c.SetPermissionError(model.PermissionManageSystem) return } - if c.App.Srv().License() != nil && c.App.Srv().License().IsCloud() { + license := c.App.Srv().License() + if license != nil && license.IsCloud() { c.Err = model.NewAppError("UploadData", "api.file.cloud_upload.app_error", nil, "", http.StatusBadRequest) return } diff --git a/server/channels/api4/user.go b/server/channels/api4/user.go index 1c0111ec8c8..10e1d339152 100644 --- a/server/channels/api4/user.go +++ b/server/channels/api4/user.go @@ -2183,7 +2183,8 @@ func loginCWS(c *Context, w http.ResponseWriter, r *http.Request) { "cyber-defense": "/cyber-defense-hq", } - if c.App.Channels().License() == nil || !c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license == nil || !license.IsCloud() { c.Err = model.NewAppError("loginCWS", "api.user.login_cws.license.error", nil, "", http.StatusUnauthorized) return } @@ -2242,7 +2243,8 @@ func loginCWS(c *Context, w http.ResponseWriter, r *http.Request) { } // If a cloud preview, redirect to the correct use case URL - if c.App.License() != nil && c.App.License().IsCloudPreview() && useCase != "" { + license = c.App.License() + if license != nil && license.IsCloudPreview() && useCase != "" { if url, ok := useCaseToURL[useCase]; ok { redirectURL += url } @@ -3185,7 +3187,8 @@ func demoteUserToGuest(c *Context, w http.ResponseWriter, r *http.Request) { return } - guestEnabled := c.App.Channels().License() != nil && *c.App.Channels().License().Features.GuestAccounts + license := c.App.Channels().License() + guestEnabled := license != nil && *license.Features.GuestAccounts if !guestEnabled { c.Err = model.NewAppError("Api4.demoteUserToGuest", "api.team.invite_guests_to_channels.disabled.error", nil, "", http.StatusForbidden) @@ -3480,7 +3483,8 @@ func migrateAuthToLDAP(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.LDAP { + license := c.App.Channels().License() + if license == nil || !*license.Features.LDAP { c.Err = model.NewAppError("api.migrateAuthToLDAP", "api.admin.ldap.not_available.app_error", nil, "", http.StatusNotImplemented) return } @@ -3539,7 +3543,8 @@ func migrateAuthToSaml(c *Context, w http.ResponseWriter, r *http.Request) { return } - if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.SAML { + license := c.App.Channels().License() + if license == nil || !*license.Features.SAML { c.Err = model.NewAppError("api.migrateAuthToSaml", "api.admin.saml.not_available.app_error", nil, "", http.StatusNotImplemented) return } diff --git a/server/channels/app/audit.go b/server/channels/app/audit.go index e0e15b7c716..8e84596b598 100644 --- a/server/channels/app/audit.go +++ b/server/channels/app/audit.go @@ -198,7 +198,8 @@ func (a *App) AddAuditLogCertificate(rctx request.CTX, fileData *multipart.FileH a.UpdateConfig(func(dest *model.Config) { *dest = *cfg }) - if a.License() != nil && a.License().IsCloud() { + license := a.License() + if license != nil && license.IsCloud() { err = a.Cloud().CreateAuditLoggingCert(rctx.Session().UserId, fileData) if err != nil { return model.NewAppError("AddAuditLogCertificate", "api.admin.add_certificate.app_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -224,7 +225,8 @@ func (a *App) RemoveAuditLogCertificate(rctx request.CTX) *model.AppError { a.UpdateConfig(func(dest *model.Config) { *dest = *cfg }) - if a.License() != nil && a.License().IsCloud() { + license := a.License() + if license != nil && license.IsCloud() { err = a.Cloud().RemoveAuditLoggingCert(rctx.Session().UserId) if err != nil { return model.NewAppError("RemoveAuditLogCertificate", "api.admin.remove_certificate.app_error", nil, "", http.StatusInternalServerError).Wrap(err) diff --git a/server/channels/app/file.go b/server/channels/app/file.go index dee2c0169f5..db8ce25812c 100644 --- a/server/channels/app/file.go +++ b/server/channels/app/file.go @@ -60,7 +60,8 @@ func (a *App) ExportFileBackend() filestore.FileBackend { func (a *App) CheckMandatoryS3Fields(settings *model.FileSettings) *model.AppError { var fileBackendSettings filestore.FileBackendSettings - if a.License() != nil && a.License().IsCloud() && a.Config().FeatureFlags.CloudDedicatedExportUI && a.Config().FileSettings.DedicatedExportStore != nil && *a.Config().FileSettings.DedicatedExportStore { + license := a.License() + if license != nil && license.IsCloud() && a.Config().FeatureFlags.CloudDedicatedExportUI && a.Config().FileSettings.DedicatedExportStore != nil && *a.Config().FileSettings.DedicatedExportStore { fileBackendSettings = filestore.NewExportFileBackendSettingsFromConfig(settings, false, false) } else { fileBackendSettings = filestore.NewFileBackendSettingsFromConfig(settings, false, false) diff --git a/server/channels/app/ip_filtering.go b/server/channels/app/ip_filtering.go index f77d3ac0e55..54d2d3b5c69 100644 --- a/server/channels/app/ip_filtering.go +++ b/server/channels/app/ip_filtering.go @@ -10,7 +10,8 @@ import ( func (a *App) SendIPFiltersChangedEmail(rctx request.CTX, userID string) error { cloudWorkspaceOwnerEmailAddress := "" - if a.License() != nil && a.License().IsCloud() { + license := a.License() + if license != nil && license.IsCloud() { portalUserCustomer, cErr := a.Cloud().GetCloudCustomer(userID) if cErr != nil { rctx.Logger().Error("Failed to get portal user customer", mlog.Err(cErr)) diff --git a/server/channels/app/login.go b/server/channels/app/login.go index b8ee21cc716..45f90950899 100644 --- a/server/channels/app/login.go +++ b/server/channels/app/login.go @@ -194,7 +194,8 @@ func (a *App) DoLogin(rctx request.CTX, w http.ResponseWriter, r *http.Request, rctx = rctx.WithSession(session) - if a.Srv().License() != nil && *a.Srv().License().Features.LDAP && a.Ldap() != nil { + license := a.Srv().License() + if license != nil && *license.Features.LDAP && a.Ldap() != nil { userVal := *user sessionVal := *session a.Srv().Go(func() { @@ -310,7 +311,8 @@ func (a *App) AttachSessionCookies(rctx request.CTX, w http.ResponseWriter, r *h http.SetCookie(w, csrfCookie) // For context see: https://mattermost.atlassian.net/browse/MM-39583 - if a.License() != nil && a.License().IsCloud() { + license := a.License() + if license != nil && license.IsCloud() { a.AttachCloudSessionCookie(rctx, w, r) } } @@ -323,5 +325,6 @@ func GetProtocol(r *http.Request) string { } func isCWSLogin(a *App, token string) bool { - return a.License() != nil && a.License().IsCloud() && token != "" + license := a.License() + return license != nil && license.IsCloud() && token != "" } diff --git a/server/channels/web/handlers.go b/server/channels/web/handlers.go index 8e0e56e5d3d..abb7b631db2 100644 --- a/server/channels/web/handlers.go +++ b/server/channels/web/handlers.go @@ -213,7 +213,8 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { subpath, _ := utils.GetSubpathFromConfig(c.App.Config()) siteURLHeader := app.GetProtocol(r) + "://" + r.Host + subpath - if c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() { + license := c.App.Channels().License() + if license != nil && license.IsCloud() { siteURLHeader = *c.App.Config().ServiceSettings.SiteURL + subpath } c.SetSiteURLHeader(siteURLHeader) @@ -285,7 +286,7 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { c.RemoveSessionCookie(w, r) c.Err = model.NewAppError("ServeHTTP", "api.context.session_expired.app_error", nil, "token="+token+" Appears to be a CSRF attempt", http.StatusUnauthorized) } - } else if token != "" && c.App.Channels().License() != nil && c.App.Channels().License().IsCloud() && tokenLocation == app.TokenLocationCloudHeader { + } else if token != "" && license != nil && license.IsCloud() && tokenLocation == app.TokenLocationCloudHeader { // Check to see if this provided token matches our CWS Token session, err := c.App.GetCloudSession(token) if err != nil { @@ -294,7 +295,7 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } else { c.AppContext = c.AppContext.WithSession(session) } - } else if token != "" && c.App.Channels().License() != nil && c.App.Channels().License().HasRemoteClusterService() && tokenLocation == app.TokenLocationRemoteClusterHeader { + } else if token != "" && license != nil && license.HasRemoteClusterService() && tokenLocation == app.TokenLocationRemoteClusterHeader { // Get the remote cluster if remoteId := c.GetRemoteID(r); remoteId == "" { c.Logger.Warn("Missing remote cluster id") // From 6527eacbb77a8cf3d6fbc573d057b1d1d088c912 Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Tue, 7 Apr 2026 17:43:57 +0000 Subject: [PATCH 04/10] fix: address CodeRabbit nil-dereference findings in cloud, team, and user handlers - Pass cached license to getPreviewSubscription to fix TOCTOU race - Add nil checks for license.Features and feature flag pointers in team.go - Add nil checks for GuestAccounts, LDAP, SAML feature pointers in user.go Co-authored-by: Claude --- server/channels/api4/cloud.go | 5 ++--- server/channels/api4/team.go | 2 +- server/channels/api4/user.go | 6 +++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/server/channels/api4/cloud.go b/server/channels/api4/cloud.go index 5c469753011..4cb19c50cac 100644 --- a/server/channels/api4/cloud.go +++ b/server/channels/api4/cloud.go @@ -65,8 +65,7 @@ func ensureCloudInterface(c *Context, where string) bool { return true } -func getPreviewSubscription(c *Context, w http.ResponseWriter, r *http.Request) { - license := c.App.Channels().License() +func getPreviewSubscription(c *Context, w http.ResponseWriter, r *http.Request, license *model.License) { subscription := &model.Subscription{ ID: "cloud-preview", ProductID: license.SkuName, @@ -92,7 +91,7 @@ func getSubscription(c *Context, w http.ResponseWriter, r *http.Request) { // Preview subscription is a special case for cloud preview licenses. license := c.App.Channels().License() if license != nil && license.IsCloudPreview() { - getPreviewSubscription(c, w, r) + getPreviewSubscription(c, w, r, license) return } diff --git a/server/channels/api4/team.go b/server/channels/api4/team.go index 3e11b6e5d1f..1294aaab603 100644 --- a/server/channels/api4/team.go +++ b/server/channels/api4/team.go @@ -1641,7 +1641,7 @@ func inviteGuestsToChannels(c *Context, w http.ResponseWriter, r *http.Request) } license := c.App.Channels().License() - guestEnabled := license != nil && *license.Features.GuestAccounts + guestEnabled := license != nil && license.Features != nil && license.Features.GuestAccounts != nil && *license.Features.GuestAccounts if !guestEnabled { c.Err = model.NewAppError("Api4.InviteGuestsToChannels", "api.team.invite_guests_to_channels.disabled.error", nil, "", http.StatusForbidden) diff --git a/server/channels/api4/user.go b/server/channels/api4/user.go index 10e1d339152..abb4f93bebf 100644 --- a/server/channels/api4/user.go +++ b/server/channels/api4/user.go @@ -3188,7 +3188,7 @@ func demoteUserToGuest(c *Context, w http.ResponseWriter, r *http.Request) { } license := c.App.Channels().License() - guestEnabled := license != nil && *license.Features.GuestAccounts + guestEnabled := license != nil && license.Features != nil && license.Features.GuestAccounts != nil && *license.Features.GuestAccounts if !guestEnabled { c.Err = model.NewAppError("Api4.demoteUserToGuest", "api.team.invite_guests_to_channels.disabled.error", nil, "", http.StatusForbidden) @@ -3484,7 +3484,7 @@ func migrateAuthToLDAP(c *Context, w http.ResponseWriter, r *http.Request) { } license := c.App.Channels().License() - if license == nil || !*license.Features.LDAP { + if license == nil || license.Features == nil || license.Features.LDAP == nil || !*license.Features.LDAP { c.Err = model.NewAppError("api.migrateAuthToLDAP", "api.admin.ldap.not_available.app_error", nil, "", http.StatusNotImplemented) return } @@ -3544,7 +3544,7 @@ func migrateAuthToSaml(c *Context, w http.ResponseWriter, r *http.Request) { } license := c.App.Channels().License() - if license == nil || !*license.Features.SAML { + if license == nil || license.Features == nil || license.Features.SAML == nil || !*license.Features.SAML { c.Err = model.NewAppError("api.migrateAuthToSaml", "api.admin.saml.not_available.app_error", nil, "", http.StatusNotImplemented) return } From cc8c406fd9988baaa2377a0ef3bce2636fafb8f3 Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Wed, 8 Apr 2026 14:56:25 +0000 Subject: [PATCH 05/10] fix: use assignment instead of short declaration for already-declared license var license was already declared earlier in addLicense(), so := on line 140 causes 'no new variables on left side of :=' compilation error. Co-authored-by: Claude --- server/channels/api4/license.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/channels/api4/license.go b/server/channels/api4/license.go index d03fdd049ab..6148974ddf6 100644 --- a/server/channels/api4/license.go +++ b/server/channels/api4/license.go @@ -137,7 +137,7 @@ func addLicense(c *Context, w http.ResponseWriter, r *http.Request) { return } - license := c.App.Channels().License() + license = c.App.Channels().License() if license != nil && license.IsCloud() { // If cloud, invalidate the caches when a new license is loaded defer func() { From 8408fef615e948f70b468115fd2d8a953ad9ee80 Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Fri, 10 Apr 2026 19:04:38 +0000 Subject: [PATCH 06/10] fix: unused license variable in addLicense handler The license return value from SaveLicense was assigned but never used after this point (SA4006 staticcheck). The parsed license from LicenseFromBytes is what gets encoded in the response. Co-authored-by: Claude --- server/channels/api4/license.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/channels/api4/license.go b/server/channels/api4/license.go index 6148974ddf6..5d24a592ead 100644 --- a/server/channels/api4/license.go +++ b/server/channels/api4/license.go @@ -124,7 +124,7 @@ func addLicense(c *Context, w http.ResponseWriter, r *http.Request) { } } - license, appErr = c.App.Srv().SaveLicense(licenseBytes) + _, appErr = c.App.Srv().SaveLicense(licenseBytes) if appErr != nil { if appErr.Id == model.ExpiredLicenseError { c.LogAudit("failed - expired or non-started license") From 9d723ee120291641a554c86152685440fdef6489 Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Fri, 17 Apr 2026 14:33:58 +0000 Subject: [PATCH 07/10] test: add nil-license regression tests for login and cloud paths Add nil-license tests for the highest-traffic code paths that were fixed but untested: - TestLoginNilLicense: exercises isCWSLogin and AttachSessionCookies with nil license via a full login flow (also hits ServeHTTP) - TestGetSubscriptionNilLicense: exercises License().IsCloudPreview() and License().IsCloud() guards in the cloud subscription handler The existing TestGetConfig/nil_license and TestCreateTeam/nil_license tests cover config and team paths. ServeHTTP is exercised implicitly by all API tests above. Co-authored-by: Claude --- server/channels/api4/cloud_test.go | 17 +++++++++++++++++ server/channels/api4/user_test.go | 16 ++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/server/channels/api4/cloud_test.go b/server/channels/api4/cloud_test.go index e3844bf01dd..67614ff68f1 100644 --- a/server/channels/api4/cloud_test.go +++ b/server/channels/api4/cloud_test.go @@ -486,3 +486,20 @@ func TestCheckCWSConnection(t *testing.T) { assert.Equal(t, "unavailable", response["status"]) }) } + +func TestGetSubscriptionNilLicense(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t) + + t.Run("nil license does not panic", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + // getSubscription calls License().IsCloudPreview() and License().IsCloud() + // — must not panic when license is nil. The exact error depends on the + // test environment (cloud interface may not be available), but the key + // assertion is no panic and no 500. + resp, err := th.SystemAdminClient.DoAPIGet(context.Background(), "/cloud/subscription", "") + require.Error(t, err) + require.NotEqual(t, http.StatusInternalServerError, resp.StatusCode) + }) +} diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go index fbf844e6330..139d473f476 100644 --- a/server/channels/api4/user_test.go +++ b/server/channels/api4/user_test.go @@ -9709,3 +9709,19 @@ func TestSearchUsersWithMfaEnforced(t *testing.T) { CheckForbiddenStatus(t, resp) }) } + +func TestLoginNilLicense(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + t.Run("nil license does not panic on login", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + _, err := th.Client.Logout(context.Background()) + require.NoError(t, err) + + // Login calls isCWSLogin and AttachSessionCookies, both of which + // reference License().IsCloud() — must not panic when license is nil. + _, _, err = th.Client.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password) + require.NoError(t, err) + }) +} From 3b0f37caea2488f0f919e066c593417c0dae3fe9 Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Fri, 17 Apr 2026 15:44:16 +0000 Subject: [PATCH 08/10] test: add nil-license regression tests for remaining 8 files Add nil-license smoke tests for: - handlers.go (ServeHTTP siteURL/CWS token) - upload.go (createUpload) - team.go (restoreTeam, importTeam) - user.go (demoteUserToGuest, migrateAuth) - license.go (addLicense) - file.go (CheckMandatoryS3Fields) - ip_filtering.go (SendIPFiltersChangedEmail) Co-authored-by: Claude --- server/channels/api4/license_test.go | 15 ++++++++ server/channels/api4/team_test.go | 46 ++++++++++++++++++++++++ server/channels/api4/upload_test.go | 19 ++++++++++ server/channels/api4/user_test.go | 42 ++++++++++++++++++++++ server/channels/app/file_test.go | 16 +++++++++ server/channels/app/ip_filtering_test.go | 26 ++++++++++++++ server/channels/web/handlers_test.go | 18 ++++++++++ 7 files changed, 182 insertions(+) create mode 100644 server/channels/app/ip_filtering_test.go diff --git a/server/channels/api4/license_test.go b/server/channels/api4/license_test.go index 958fc12092d..17a8d9e3fcb 100644 --- a/server/channels/api4/license_test.go +++ b/server/channels/api4/license_test.go @@ -618,3 +618,18 @@ func TestGetLicenseLoadMetric(t *testing.T) { require.Equal(t, 1500, loadValue) }) } + +func TestAddLicenseNilLicense(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + t.Run("nil license does not panic", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + // addLicense checks License().IsCloud() after saving — but the upload + // itself will fail validation. Key assertion: no panic, no 500. + resp, err := th.SystemAdminClient.DoAPIPost(context.Background(), "/license", "not-a-real-license") + require.Error(t, err) + require.NotEqual(t, http.StatusInternalServerError, resp.StatusCode) + }) +} diff --git a/server/channels/api4/team_test.go b/server/channels/api4/team_test.go index f29d782212a..115047e8062 100644 --- a/server/channels/api4/team_test.go +++ b/server/channels/api4/team_test.go @@ -4,11 +4,13 @@ package api4 import ( + "bytes" "context" "encoding/base64" "encoding/binary" "encoding/json" "fmt" + "mime/multipart" "net/http" "strconv" "strings" @@ -4868,3 +4870,47 @@ func TestGetTeamMembersForUserRoleDataSanitization(t *testing.T) { require.Fail(t, "basic team membership not found") }) } + +func TestRestoreTeamNilLicense(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + t.Run("nil license does not panic", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + // restoreTeam checks License().IsCloud() — must not panic when nil. + resp, err := th.SystemAdminClient.DoAPIPost(context.Background(), "/teams/"+th.BasicTeam.Id+"/restore", "") + // May return 403/404 depending on team state, but must not return 500. + if err != nil { + require.NotEqual(t, http.StatusInternalServerError, resp.StatusCode) + } + }) +} + +func TestImportTeamNilLicense(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + t.Run("nil license does not panic", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + // importTeam checks License().IsCloud() — must not panic when nil. + // We send a minimal multipart request that will fail validation but should not panic. + var b bytes.Buffer + w := multipart.NewWriter(&b) + w.WriteField("importFrom", "slack") + w.WriteField("filesize", "100") + part, _ := w.CreateFormFile("file", "import.zip") + part.Write([]byte("fake")) + w.Close() + + req, _ := http.NewRequest("POST", th.SystemAdminClient.APIURL+"/teams/"+th.BasicTeam.Id+"/import", &b) + req.Header.Set("Content-Type", w.FormDataContentType()) + req.Header.Set(model.HeaderAuth, model.HeaderBearer+" "+th.SystemAdminClient.AuthToken) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + resp.Body.Close() + require.NotEqual(t, http.StatusInternalServerError, resp.StatusCode, + "nil license must not cause a 500 panic in importTeam") + }) +} diff --git a/server/channels/api4/upload_test.go b/server/channels/api4/upload_test.go index 2c99f7e47c0..b5b5fac2d3e 100644 --- a/server/channels/api4/upload_test.go +++ b/server/channels/api4/upload_test.go @@ -551,3 +551,22 @@ func TestUploadDataMultipart(t *testing.T) { require.Equal(t, file, data) }) } + +func TestCreateUploadNilLicense(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + t.Run("nil license does not panic on import upload", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + us := &model.UploadSession{ + Type: model.UploadTypeImport, + Filename: "import.zip", + FileSize: 1024, + } + _, resp, err := th.SystemAdminClient.CreateUpload(context.Background(), us) + require.Error(t, err) + require.NotEqual(t, http.StatusInternalServerError, resp.StatusCode, + "nil license must not cause a 500 panic in createUpload") + }) +} diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go index 139d473f476..af9bc42b53a 100644 --- a/server/channels/api4/user_test.go +++ b/server/channels/api4/user_test.go @@ -9725,3 +9725,45 @@ func TestLoginNilLicense(t *testing.T) { require.NoError(t, err) }) } + +func TestDemoteUserToGuestNilLicense(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + t.Run("nil license does not panic", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + // demoteUserToGuest checks License() == nil — should return 501, not panic. + resp, err := th.SystemAdminClient.DoAPIPost(context.Background(), "/users/"+th.BasicUser.Id+"/demote", "") + require.Error(t, err) + require.NotEqual(t, http.StatusInternalServerError, resp.StatusCode) + }) +} + +func TestMigrateAuthToLDAPNilLicense(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + t.Run("nil license does not panic", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + jsonBody := `{"from":"email","force":false,"match_field":"email"}` + resp, err := th.SystemAdminClient.DoAPIPost(context.Background(), "/users/migrate_auth/ldap", jsonBody) + require.Error(t, err) + require.NotEqual(t, http.StatusInternalServerError, resp.StatusCode) + }) +} + +func TestMigrateAuthToSamlNilLicense(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + t.Run("nil license does not panic", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + jsonBody := `{"from":"email","auto":false,"matches":{}}` + resp, err := th.SystemAdminClient.DoAPIPost(context.Background(), "/users/migrate_auth/saml", jsonBody) + require.Error(t, err) + require.NotEqual(t, http.StatusInternalServerError, resp.StatusCode) + }) +} diff --git a/server/channels/app/file_test.go b/server/channels/app/file_test.go index e13f43b393a..3bd9a3ec887 100644 --- a/server/channels/app/file_test.go +++ b/server/channels/app/file_test.go @@ -1151,3 +1151,19 @@ func TestFilterFilesByChannelPermissions_ABAC(t *testing.T) { mockACS.AssertNotCalled(t, "AccessEvaluation") }) } + +func TestCheckMandatoryS3FieldsNilLicense(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t) + defer th.TearDown() + + t.Run("nil license does not panic", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + settings := &model.FileSettings{} + settings.SetDefaults(false) + // CheckMandatoryS3Fields calls License().IsCloud() — must not panic when nil. + _ = th.App.CheckMandatoryS3Fields(settings) + // If we get here without a panic, the test passes. + }) +} diff --git a/server/channels/app/ip_filtering_test.go b/server/channels/app/ip_filtering_test.go new file mode 100644 index 00000000000..f0a8b7dcdbb --- /dev/null +++ b/server/channels/app/ip_filtering_test.go @@ -0,0 +1,26 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/request" +) + +func TestSendIPFiltersChangedEmailNilLicense(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t) + defer th.TearDown() + + t.Run("nil license does not panic", func(t *testing.T) { + th.App.Srv().SetLicense(nil) + + rctx := request.TestContext(t) + // SendIPFiltersChangedEmail checks License().IsCloud() — must not panic when nil. + // It may return an error (e.g., no SMTP configured), but must not panic. + _ = th.App.SendIPFiltersChangedEmail(rctx, model.NewId()) + }) +} diff --git a/server/channels/web/handlers_test.go b/server/channels/web/handlers_test.go index 456c3f7c14b..2cc85b9c705 100644 --- a/server/channels/web/handlers_test.go +++ b/server/channels/web/handlers_test.go @@ -1179,3 +1179,21 @@ func TestHandleContextErrorZeroStatusCode(t *testing.T) { assert.Equal(t, http.StatusBadRequest, response.Code) }) } + +func TestHandlerServeHTTPNilLicense(t *testing.T) { + th := Setup(t) + + th.App.Srv().SetLicense(nil) + + web := New(th.Server) + handler := web.NewHandler(handlerForServeDefaultSecurityHeaders) + + // ServeHTTP checks License().IsCloud() for siteURL and CWS token — must not panic when nil. + request := httptest.NewRequest("GET", "/api/v4/test", nil) + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + + // Should complete without panic. Any non-500 status is acceptable. + require.NotEqual(t, http.StatusInternalServerError, response.Code, + "nil license must not cause a 500 panic in ServeHTTP") +} From b59df45c045d69b29ed7ee2efacb754808b3680e Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Fri, 17 Apr 2026 16:07:43 +0000 Subject: [PATCH 09/10] fix(test): remove TearDown calls from app package tests The app package TestHelper uses t.Cleanup internally and does not expose a TearDown method. Co-authored-by: Claude --- server/channels/app/file_test.go | 1 - server/channels/app/ip_filtering_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/server/channels/app/file_test.go b/server/channels/app/file_test.go index 3bd9a3ec887..adbc18b3211 100644 --- a/server/channels/app/file_test.go +++ b/server/channels/app/file_test.go @@ -1155,7 +1155,6 @@ func TestFilterFilesByChannelPermissions_ABAC(t *testing.T) { func TestCheckMandatoryS3FieldsNilLicense(t *testing.T) { mainHelper.Parallel(t) th := Setup(t) - defer th.TearDown() t.Run("nil license does not panic", func(t *testing.T) { th.App.Srv().SetLicense(nil) diff --git a/server/channels/app/ip_filtering_test.go b/server/channels/app/ip_filtering_test.go index f0a8b7dcdbb..34d30b47395 100644 --- a/server/channels/app/ip_filtering_test.go +++ b/server/channels/app/ip_filtering_test.go @@ -13,7 +13,6 @@ import ( func TestSendIPFiltersChangedEmailNilLicense(t *testing.T) { mainHelper.Parallel(t) th := Setup(t) - defer th.TearDown() t.Run("nil license does not panic", func(t *testing.T) { th.App.Srv().SetLicense(nil) From 4b527ba3ad60a1a1cf03d14da952a89d57964062 Mon Sep 17 00:00:00 2001 From: Pavel Zeman Date: Fri, 17 Apr 2026 16:47:15 +0000 Subject: [PATCH 10/10] fix(test): check error returns in TestImportTeamNilLicense Fixes errcheck lint violations for WriteField, CreateFormFile, and Write calls in the import team nil-license test. Co-authored-by: Claude --- server/channels/api4/team_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/server/channels/api4/team_test.go b/server/channels/api4/team_test.go index 115047e8062..d89fe1b72ec 100644 --- a/server/channels/api4/team_test.go +++ b/server/channels/api4/team_test.go @@ -4898,10 +4898,12 @@ func TestImportTeamNilLicense(t *testing.T) { // We send a minimal multipart request that will fail validation but should not panic. var b bytes.Buffer w := multipart.NewWriter(&b) - w.WriteField("importFrom", "slack") - w.WriteField("filesize", "100") - part, _ := w.CreateFormFile("file", "import.zip") - part.Write([]byte("fake")) + require.NoError(t, w.WriteField("importFrom", "slack")) + require.NoError(t, w.WriteField("filesize", "100")) + part, err := w.CreateFormFile("file", "import.zip") + require.NoError(t, err) + _, err = part.Write([]byte("fake")) + require.NoError(t, err) w.Close() req, _ := http.NewRequest("POST", th.SystemAdminClient.APIURL+"/teams/"+th.BasicTeam.Id+"/import", &b)