From 71bb999950ad01014d1482e166c273c83b54b8af Mon Sep 17 00:00:00 2001 From: jamesread Date: Sun, 8 Mar 2026 22:45:46 +0000 Subject: [PATCH] security: Actions that people didnt have permission to view were being returned (#921) --- service/internal/api/api_test.go | 67 ++++++++++++++++++++++ service/internal/api/dashboard_entities.go | 8 ++- service/internal/api/dashboards.go | 19 +++++- 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/service/internal/api/api_test.go b/service/internal/api/api_test.go index 7bca057..57abddd 100644 --- a/service/internal/api/api_test.go +++ b/service/internal/api/api_test.go @@ -432,6 +432,73 @@ func TestViewPermissionAllowedSeesAction(t *testing.T) { assert.Equal(t, "secret_action", resp.Action.BindingId) } +// TestViewPermissionExcludedFromCustomDashboard (issue #921) asserts that when a custom dashboard +// lists an action by title, users without view permission do not see that action (title or icon). +func TestViewPermissionExcludedFromCustomDashboard(t *testing.T) { + cfg, lowUser, _ := buildViewPermissionTestConfig(t) + cfg.Dashboards = []*config.DashboardComponent{ + { + Title: "Custom", + Contents: []*config.DashboardComponent{ + {Title: "Secret Action"}, + }, + }, + } + ex := executor.DefaultExecutor(cfg) + ex.RebuildActionMap() + + rr := &DashboardRenderRequest{ + AuthenticatedUser: lowUser, + cfg: cfg, + ex: ex, + } + dashboard := findDashboardByTitle(rr, "Custom") + require.NotNil(t, dashboard) + db := buildDashboardFromConfig(dashboard, rr) + require.NotNil(t, db) + + bindingIdsInDashboard := bindingIdsInDashboardContents(db.Contents) + assert.NotContains(t, bindingIdsInDashboard, "secret_action", + "user with view:false must not see action on custom dashboard; got bindingIds: %v", bindingIdsInDashboard) +} + +// TestViewPermissionExcludedFromEntityDashboard (GHSA: view permission) asserts that when a dashboard +// has an entity fieldset listing an action, users without view permission do not see that action. +func TestViewPermissionExcludedFromEntityDashboard(t *testing.T) { + entities.ClearEntitiesOfType("vp_entity_test") + defer entities.ClearEntitiesOfType("vp_entity_test") + entities.AddEntity("vp_entity_test", "1", map[string]any{"title": "Test Entity"}) + + cfg, lowUser, _ := buildViewPermissionTestConfig(t) + cfg.Dashboards = []*config.DashboardComponent{ + { + Title: "WithEntity", + Contents: []*config.DashboardComponent{ + { + Title: "Servers", Type: "fieldset", Entity: "vp_entity_test", + Contents: []*config.DashboardComponent{{Title: "Secret Action"}}, + }, + }, + }, + } + ex := executor.DefaultExecutor(cfg) + ex.RebuildActionMap() + + rr := &DashboardRenderRequest{ + AuthenticatedUser: lowUser, + cfg: cfg, + ex: ex, + } + dashboard := findDashboardByTitle(rr, "WithEntity") + require.NotNil(t, dashboard) + db := buildDashboardFromConfig(dashboard, rr) + require.NotNil(t, db) + + bindingIdsInDashboard := bindingIdsInDashboardContents(db.Contents) + assert.NotContains(t, bindingIdsInDashboard, "secret_action", + "user with view:false must not see action in entity fieldset; got bindingIds: %v", bindingIdsInDashboard) +} + func bindingIdsInDashboardContents(contents []*apiv1.DashboardComponent) []string { var ids []string for _, c := range contents { diff --git a/service/internal/api/dashboard_entities.go b/service/internal/api/dashboard_entities.go index d9a12d6..d588a57 100644 --- a/service/internal/api/dashboard_entities.go +++ b/service/internal/api/dashboard_entities.go @@ -82,8 +82,6 @@ func isLinkType(itemType string) bool { } func cloneLinkItem(subitem *config.DashboardComponent, ent *entities.Entity, clone *apiv1.DashboardComponent, rr *DashboardRenderRequest) *apiv1.DashboardComponent { - clone.Type = "link" - clone.Title = tpl.ParseTemplateOfActionBeforeExec(subitem.Title, ent) // Prefer an entity-specific action when available, but fall back to a // non-entity-scoped action with the same title. This allows inline actions // defined inside entity dashboards to work without requiring an explicit @@ -92,7 +90,11 @@ func cloneLinkItem(subitem *config.DashboardComponent, ent *entities.Entity, clo if action == nil { action = rr.findAction(subitem.Title) } - + if action == nil { + return nil + } + clone.Type = "link" + clone.Title = tpl.ParseTemplateOfActionBeforeExec(subitem.Title, ent) clone.Action = action return clone } diff --git a/service/internal/api/dashboards.go b/service/internal/api/dashboards.go index 4e98fde..44e2244 100644 --- a/service/internal/api/dashboards.go +++ b/service/internal/api/dashboards.go @@ -277,16 +277,31 @@ func createRootFieldset() *apiv1.DashboardComponent { } } +func appendComponentIfNotNil(components *[]*apiv1.DashboardComponent, comp *apiv1.DashboardComponent) { + if comp != nil { + *components = append(*components, comp) + } +} + +func getDashboardComponentOrNil(subitem *config.DashboardComponent, rr *DashboardRenderRequest, entity *entities.Entity) *apiv1.DashboardComponent { + if len(subitem.Contents) == 0 && rr.findActionForEntity(subitem.Title, entity) == nil { + if !isAllowedType(subitem.Type) { + return nil + } + } + return buildDashboardComponentSimpleWithEntity(subitem, rr, entity) +} + func processDashboardSubitemWithEntity(subitem *config.DashboardComponent, rr *DashboardRenderRequest, ret *[]*apiv1.DashboardComponent, rootFieldset *apiv1.DashboardComponent, entity *entities.Entity) { if subitem.Type != "fieldset" { - rootFieldset.Contents = append(rootFieldset.Contents, buildDashboardComponentSimpleWithEntity(subitem, rr, entity)) + appendComponentIfNotNil(&rootFieldset.Contents, getDashboardComponentOrNil(subitem, rr, entity)) return } if subitem.Entity != "" { *ret = append(*ret, buildEntityFieldsets(subitem.Entity, subitem, rr)...) } else { - *ret = append(*ret, buildDashboardComponentSimpleWithEntity(subitem, rr, entity)) + appendComponentIfNotNil(ret, getDashboardComponentOrNil(subitem, rr, entity)) } }