public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Alerting: Prevent users from saving rules to git-synced folders (#114944)

Browse Source 
---------

Co-authored-by: Yuri Tseretyan <yuriy.tseretyan@grafana.com>
This commit is contained in:
William Wernert
2025-12-12 15:25:08 +00:00
committed by GitHub
co-authored by Yuri Tseretyan
parent eab5d2b30e
commit 5f80a29a28
8 changed files with 209 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/ngalert/api/api_ruler.go
+11 -2
View File
@@ -412,11 +412,16 @@ func (srv RulerSrv) RoutePostNameRulesConfig(c *contextmodel.ReqContext, ruleGro
deletePermanently = true
}
namespace, err := srv.store.GetNamespaceByUID(c.Req.Context(), namespaceUID, c.GetOrgID(), c.SignedInUser)
f, err := srv.store.GetNamespaceByUID(c.Req.Context(), namespaceUID, c.GetOrgID(), c.SignedInUser)
if err != nil {
return toNamespaceErrorResponse(err)
}
namespace := ngmodels.NewNamespace(f)
if err := namespace.ValidateForRuleStorage(); err != nil {
return ErrResp(http.StatusBadRequest, fmt.Errorf("%w: %s", ngmodels.ErrAlertRuleFailedValidation, err), "")
}
if err := srv.checkGroupLimits(ruleGroupConfig); err != nil {
return ErrResp(http.StatusBadRequest, err, "")
}
@@ -841,10 +846,14 @@ func (srv RulerSrv) RouteUpdateNamespaceRules(c *contextmodel.ReqContext, body a
return ErrResp(http.StatusBadRequest, errors.New("missing request body"), "")
}
namespace, err := srv.store.GetNamespaceByUID(c.Req.Context(), namespaceUID, c.GetOrgID(), c.SignedInUser)
f, err := srv.store.GetNamespaceByUID(c.Req.Context(), namespaceUID, c.GetOrgID(), c.SignedInUser)
if err != nil {
return toNamespaceErrorResponse(err)
}
namespace := ngmodels.NewNamespace(f)
if err := namespace.ValidateForRuleStorage(); err != nil {
return ErrResp(http.StatusBadRequest, fmt.Errorf("%w: %s", ngmodels.ErrAlertRuleFailedValidation, err), "")
}
ruleGroups, _, err := srv.searchAuthorizedAlertRules(c.Req.Context(), authorizedRuleGroupQuery{
User: c.SignedInUser,
pkg/services/ngalert/api/api_ruler_test.go
+61
View File
@@ -18,6 +18,7 @@ import (
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/apimachinery/utils"
"github.com/grafana/grafana/pkg/infra/log"
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
@@ -1288,4 +1289,64 @@ func TestRouteUpdateNamespaceRules(t *testing.T) {
updatedRules := getRecordedUpdatedRules(ruleStore)
require.Empty(t, updatedRules)
})
t.Run("should reject update when folder is managed by ManagerKindRepo", func(t *testing.T) {
ruleStore := fakes.NewRuleStore(t)
provisioningStore := fakes.NewFakeProvisioningStore()
// Create a managed folder
managedFolder := randFolder()
managedFolder.ManagedBy = utils.ManagerKindRepo
ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], managedFolder)
// Create some rules in the managed folder
ruleGen := models.RuleGen.With(
models.RuleGen.WithOrgID(orgID),
models.RuleGen.WithNamespaceUID(managedFolder.UID),
)
rules := ruleGen.GenerateManyRef(2)
ruleStore.PutRule(context.Background(), rules...)
permissions := createPermissionsForRules(rules, orgID)
requestCtx := createRequestContextWithPerms(orgID, permissions, nil)
svc := createServiceWithProvenanceStore(ruleStore, provisioningStore)
response := svc.RouteUpdateNamespaceRules(requestCtx, apimodels.UpdateNamespaceRulesRequest{
IsPaused: util.Pointer(true),
}, managedFolder.UID)
require.Equal(t, http.StatusBadRequest, response.Status())
require.Contains(t, string(response.Body()), "cannot store rules in folder managed by Git Sync")
// Verify no rules were updated
updatedRules := getRecordedUpdatedRules(ruleStore)
require.Empty(t, updatedRules)
})
}
func TestRoutePostNameRulesConfig(t *testing.T) {
t.Run("should reject creation when folder is managed by ManagerKindRepo", func(t *testing.T) {
orgID := rand.Int63()
ruleStore := fakes.NewRuleStore(t)
// Create a managed folder
managedFolder := randFolder()
managedFolder.ManagedBy = utils.ManagerKindRepo
ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], managedFolder)
permissions := map[int64]map[string][]string{
orgID: {
dashboards.ScopeFoldersProvider.GetResourceScopeUID(managedFolder.UID): {dashboards.ActionFoldersRead},
},
}
requestCtx := createRequestContextWithPerms(orgID, permissions, nil)
svc := createService(ruleStore, nil)
response := svc.RoutePostNameRulesConfig(requestCtx, apimodels.PostableRuleGroupConfig{
Name: "test-group",
}, managedFolder.UID)
require.Equal(t, http.StatusBadRequest, response.Status())
require.Contains(t, string(response.Body()), "cannot store rules in folder managed by Git Sync")
})
}
pkg/services/ngalert/api/prometheus/api_prometheus.go
+1 -1
View File
@@ -296,7 +296,7 @@ func (srv PrometheusSrv) RouteGetRuleStatuses(c *contextmodel.ReqContext) respon
allowedNamespaces := map[string]string{}
for namespaceUID, folder := range namespaceMap {
// only add namespaces that the user has access to rules in
hasAccess, err := srv.authz.HasAccessInFolder(c.Req.Context(), c.SignedInUser, ngmodels.Namespace(*folder.ToFolderReference()))
hasAccess, err := srv.authz.HasAccessInFolder(c.Req.Context(), c.SignedInUser, ngmodels.NewNamespace(folder))
if err != nil {
ruleResponse.Status = "error"
ruleResponse.Error = fmt.Sprintf("failed to get namespaces visible to the user: %s", err.Error())
pkg/services/ngalert/eval/eval.go
+1 -1
View File
@@ -204,7 +204,7 @@ func IsNonRetryableError(err error) bool {
return false
}
// HasErrors returns true when Results contains at least one element and all elements are errors
// IsError returns true when Results contains at least one element and all elements are errors
func (evalResults Results) IsError() bool {
for _, r := range evalResults {
if r.State != Error {
pkg/services/ngalert/models/alert_rule.go
+15
View File
@@ -24,6 +24,7 @@ import (
alertingModels "github.com/grafana/alerting/models"
"github.com/grafana/grafana/pkg/apimachinery/utils"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/quota"
"github.com/grafana/grafana/pkg/setting"
@@ -397,6 +398,20 @@ type Namespaced interface {
type Namespace folder.FolderReference
func NewNamespace(f *folder.Folder) Namespace {
return Namespace(*f.ToFolderReference())
}
func (n Namespace) ValidateForRuleStorage() error {
if n.UID == "" {
return fmt.Errorf("cannot store rules in folder without UID")
}
if n.ManagedBy == utils.ManagerKindRepo {
return fmt.Errorf("cannot store rules in folder managed by Git Sync")
}
return nil
}
func (n Namespace) GetNamespaceUID() string {
return n.UID
}
pkg/services/ngalert/provisioning/alert_rules.go
+16 -3
View File
@@ -114,7 +114,7 @@ func (service *AlertRuleService) ListAlertRules(ctx context.Context, user identi
}
folderUIDs := make([]string, 0, len(folders))
for _, f := range folders {
access, err := service.authz.HasAccessInFolder(ctx, user, models.Namespace(*f.ToFolderReference()))
access, err := service.authz.HasAccessInFolder(ctx, user, models.NewNamespace(f))
if err != nil {
return nil, nil, "", err
}
@@ -407,6 +407,9 @@ func (service *AlertRuleService) UpdateRuleGroup(ctx context.Context, user ident
if err := models.ValidateRuleGroupInterval(intervalSeconds, service.baseIntervalSeconds); err != nil {
return err
}
if err := service.ensureNamespace(ctx, user, user.GetOrgID(), namespaceUID); err != nil {
return err
}
return service.xact.InTransaction(ctx, func(ctx context.Context) error {
query := &models.ListAlertRulesQuery{
OrgID: user.GetOrgID(),
@@ -471,6 +474,10 @@ func (service *AlertRuleService) ReplaceRuleGroup(ctx context.Context, user iden
return err
}
if err := service.ensureNamespace(ctx, user, user.GetOrgID(), group.FolderUID); err != nil {
return err
}
// If the rule group is reserved for no-group rules, we cannot have multiple rules in it.
if models.IsNoGroupRuleGroup(group.Title) && len(group.Rules) > 1 {
return fmt.Errorf("rule group %s is reserved for no-group rules and cannot be used for rule groups with multiple rules", group.Title)
@@ -1025,6 +1032,7 @@ func (service *AlertRuleService) checkGroupLimits(group models.AlertRuleGroup) e
// ensureNamespace ensures that the rule has a valid namespace UID.
// If the rule does not have a namespace UID or the namespace (folder) does not exist it will return an error.
// If the folder is managed by a manager, it will also return an error.
func (service *AlertRuleService) ensureNamespace(ctx context.Context, user identity.Requester, orgID int64, namespaceUID string) error {
if namespaceUID == "" {
return fmt.Errorf("%w: folderUID must be set", models.ErrAlertRuleFailedValidation)
@@ -1037,18 +1045,23 @@ func (service *AlertRuleService) ensureNamespace(ctx context.Context, user ident
}
// ensure the namespace exists
_, err := service.folderService.Get(ctx, &folder.GetFolderQuery{
f, err := service.folderService.Get(ctx, &folder.GetFolderQuery{
OrgID: orgID,
UID: &namespaceUID,
SignedInUser: user,
})
if err != nil {
if err != nil || f == nil {
if errors.Is(err, dashboards.ErrFolderNotFound) {
return fmt.Errorf("%w: folder does not exist", models.ErrAlertRuleFailedValidation)
}
return err
}
// check if the folder is managed by a manager
if err := models.NewNamespace(f).ValidateForRuleStorage(); err != nil {
return fmt.Errorf("%w: %s", models.ErrAlertRuleFailedValidation, err)
}
return nil
}
pkg/services/ngalert/provisioning/alert_rules_test.go
+103
View File
@@ -16,6 +16,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/apimachinery/utils"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/expr"
"github.com/grafana/grafana/pkg/infra/db"
@@ -867,6 +868,27 @@ func TestIntegrationAlertRuleService(t *testing.T) {
require.NoError(t, err)
require.Equal(t, int64(120), rule.IntervalSeconds)
})
t.Run("UpdateRuleGroup should reject when folder is managed by a manager", func(t *testing.T) {
service, _, _, ac := initService(t)
ac.CanWriteAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) {
return true, nil
}
managedFolderUID := "managed-folder-update-group"
fs := foldertest.NewFakeService()
fs.AddFolder(&folder.Folder{
OrgID: orgID,
UID: managedFolderUID,
Title: "Managed Folder",
ManagedBy: utils.ManagerKindRepo,
})
service.folderService = fs
err := service.UpdateRuleGroup(context.Background(), u, managedFolderUID, "some-group", 120)
require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation)
require.ErrorContains(t, err, "cannot store rules in folder managed by Git Sync")
})
}
func TestIntegrationCreateAlertRule(t *testing.T) {
@@ -1166,6 +1188,30 @@ func TestIntegrationCreateAlertRule(t *testing.T) {
require.NoError(t, err)
require.True(t, models.IsNoGroupRuleGroup(retrievedRule.RuleGroup), "Rule should be considered NoGroup rule")
})
t.Run("should reject creation when folder is managed by a manager", func(t *testing.T) {
service, _, _, ac := initService(t)
ac.CanWriteAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) {
return true, nil
}
managedFolderUID := "managed-folder"
fs := foldertest.NewFakeService()
fs.AddFolder(&folder.Folder{
OrgID: orgID,
UID: managedFolderUID,
Title: "Managed Folder",
ManagedBy: utils.ManagerKindRepo,
})
service.folderService = fs
rule := dummyRule("test-managed-folder", orgID)
rule.NamespaceUID = managedFolderUID
_, err := service.CreateAlertRule(context.Background(), u, rule, models.ProvenanceNone)
require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation)
require.ErrorContains(t, err, "cannot store rules in folder managed by Git Sync")
})
}
func TestUpdateAlertRule(t *testing.T) {
@@ -1316,6 +1362,36 @@ func TestUpdateAlertRule(t *testing.T) {
require.Equal(t, "nogroup-update-new", updated.Title)
require.Equal(t, originalInterval, updated.IntervalSeconds)
})
t.Run("should reject update when folder is managed by a manager", func(t *testing.T) {
service, ruleStore, provenanceStore, ac := initService(t)
ac.CanWriteAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) {
return true, nil
}
managedFolderUID := "managed-folder-update"
fs := foldertest.NewFakeService()
fs.AddFolder(&folder.Folder{
OrgID: orgID,
UID: managedFolderUID,
Title: "Managed Folder",
ManagedBy: utils.ManagerKindRepo,
})
service.folderService = fs
// Create an existing rule
existingRule := dummyRule("test-managed-folder-update", orgID)
existingRule.NamespaceUID = managedFolderUID
_, err := ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.InsertRule{{AlertRule: existingRule}})
require.NoError(t, err)
require.NoError(t, provenanceStore.SetProvenance(context.Background(), &existingRule, orgID, models.ProvenanceNone))
// Try to update the rule
existingRule.Title = "Updated Title"
_, err = service.UpdateAlertRule(context.Background(), u, existingRule, models.ProvenanceNone)
require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation)
require.ErrorContains(t, err, "cannot store rules in folder managed by Git Sync")
})
}
func TestDeleteAlertRule(t *testing.T) {
@@ -2054,6 +2130,33 @@ func TestReplaceGroup(t *testing.T) {
require.Error(t, err)
require.ErrorContains(t, err, "cannot move rule out of this group")
})
t.Run("should reject replace when folder is managed by a manager", func(t *testing.T) {
service, _, _, ac := initService(t)
ac.CanWriteAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) {
return true, nil
}
managedFolderUID := "managed-folder-replace"
fs := foldertest.NewFakeService()
fs.AddFolder(&folder.Folder{
OrgID: orgID,
UID: managedFolderUID,
Title: "Managed Folder",
ManagedBy: utils.ManagerKindRepo,
})
service.folderService = fs
group := models.AlertRuleGroup{
Title: "test-group",
FolderUID: managedFolderUID,
Interval: 60,
}
err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceNone, "")
require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation)
require.ErrorContains(t, err, "cannot store rules in folder managed by Git Sync")
})
}
func TestDeleteRuleGroup(t *testing.T) {
pkg/services/ngalert/state/historian/loki.go
+1 -1
View File
@@ -530,7 +530,7 @@ func (h *RemoteLokiBackend) getFolderUIDsForFilter(ctx context.Context, query mo
uids := make([]string, 0, len(folders))
// now keep only UIDs of folder in which user can read rules.
for _, f := range folders {
hasAccess, err := h.ac.HasAccessInFolder(ctx, query.SignedInUser, models.Namespace(*f.ToFolderReference()))
hasAccess, err := h.ac.HasAccessInFolder(ctx, query.SignedInUser, models.NewNamespace(f))
if err != nil {
return nil, err
}