medlogic
/
olivetin
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

chore: coderabbit suggestions

This commit is contained in:
jamesread 2026-08-11 10:24:38 +01:00
parent 27a451409e
commit 33e2ab3a37
8 changed files with 216 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
frontend/resources/vue/stores/searchIndex.js
View File

@ -73,10 +73,10 @@ export function dashboardRoutePath (title, entityType, entityKey) {
return '/'
}
let path = `/dashboards/${title}`
let path = `/dashboards/${encodeURIComponent(title)}`
if (entityType && entityKey) {
path += `/${entityType}/${entityKey}`
path += `/${encodeURIComponent(entityType)}/${encodeURIComponent(entityKey)}`
}
return path
@ -123,7 +123,7 @@ function entityItemsFromHints (hints) {
description: hint.type,
category: 'Entities',
type: 'route',
path: `/entity-details/${hint.type}/${hint.uniqueKey}`,
path: `/entity-details/${encodeURIComponent(hint.type)}/${encodeURIComponent(hint.uniqueKey)}`,
icon: CellsIcon
})
}
@ -147,7 +147,7 @@ function actionItemsFromHints (hints) {
title: hint.title || hint.bindingId,
category: 'Actions',
type: 'route',
path: `/action/${hint.bindingId}`,
path: `/action/${encodeURIComponent(hint.bindingId)}`,
icon: PlayIcon
})
}

30
frontend/resources/vue/stores/searchIndex.test.mjs
View File

@ -27,6 +27,14 @@ test('dashboardRoutePath builds dashboard and entity paths', () => {
dashboardRoutePath('Servers', 'host', 'web01'),
'/dashboards/Servers/host/web01'
)
assert.equal(
dashboardRoutePath('Ops/Prod', 'host/type', 'key?1'),
'/dashboards/Ops%2FProd/host%2Ftype/key%3F1'
)
assert.equal(
dashboardRoutePath('Servers', 'host', 'key#frag'),
'/dashboards/Servers/host/key%23frag'
)
})
test('indexSearchHints indexes entities and actions', () => {
@ -36,12 +44,17 @@ test('indexSearchHints indexes entities and actions', () => {
entities: [
{ title: 'web01', type: 'host', uniqueKey: '0' },
{ title: '', type: 'host', uniqueKey: '1' },
{ title: 'skip', type: '', uniqueKey: 'x' }
{ title: 'skip', type: '', uniqueKey: 'x' },
{ title: 'slash host', type: 'host/type', uniqueKey: 'key?1' },
{ title: 'hash host', type: 'host', uniqueKey: 'key#frag' }
],
actions: [
{ title: 'Ping Host', bindingId: 'bind-ping' },
{ title: '', bindingId: 'bind-empty-title' },
{ title: 'Ignored', bindingId: '' }
{ title: 'Ignored', bindingId: '' },
{ title: 'Slash Action', bindingId: 'bind/slash' },
{ title: 'Query Action', bindingId: 'bind?query' },
{ title: 'Hash Action', bindingId: 'bind#hash' }
]
})
@ -51,9 +64,20 @@ test('indexSearchHints indexes entities and actions', () => {
assert.equal(byId['entity:host:0'].path, '/entity-details/host/0')
assert.equal(byId['entity:host:0'].description, 'host')
assert.equal(byId['entity:host:1'].title, '1')
assert.equal(
byId['entity:host/type:key?1'].path,
'/entity-details/host%2Ftype/key%3F1'
)
assert.equal(
byId['entity:host:key#frag'].path,
'/entity-details/host/key%23frag'
)
assert.equal(byId['action:bind-ping'].title, 'Ping Host')
assert.equal(byId['action:bind-ping'].path, '/action/bind-ping')
assert.equal(byId['action:bind/slash'].path, '/action/bind%2Fslash')
assert.equal(byId['action:bind?query'].path, '/action/bind%3Fquery')
assert.equal(byId['action:bind#hash'].path, '/action/bind%23hash')
assert.equal(byId['action:bind-ping'].category, 'Actions')
assert.equal(byId['action:bind-empty-title'].title, 'bind-empty-title')
@ -72,7 +96,7 @@ test('indexRootDashboardEntries indexes ACL-filtered dashboards', () => {
const byId = Object.fromEntries(searchIndexItems.value.map((item) => [item.id, item]))
assert.equal(byId['dashboard:Actions'].path, '/')
assert.equal(byId['dashboard:My Server'].path, '/dashboards/My Server')
assert.equal(byId['dashboard:My Server'].path, '/dashboards/My%20Server')
assert.equal(byId['dashboard:My Server'].description, 'Infrastructure')
assert.equal(byId['dashboard:My Server'].category, 'Dashboards')
})

49
service/internal/api/api_entity_argument_acl.go
View File

@ -34,8 +34,8 @@ func (api *oliveTinAPI) errUnlessEntityArgumentsAllowed(user *authpublic.Authent
return nil
}
for i := range action.Arguments {
arg := &action.Arguments[i]
for argumentIndex := range action.Arguments {
arg := &action.Arguments[argumentIndex]
if arg.Entity == "" {
continue
}
@ -108,10 +108,11 @@ func errUnlessEntityArgumentValueAllowed(arg *config.ActionArgument, value strin
func entityArgumentValueAllowed(arg *config.ActionArgument, value string) bool {
allowed := entityArgumentAllowedValues(arg)
if strings.EqualFold(arg.Type, "checklist") {
return checklistEntityValuesAllowed(value, allowed)
return checklistEntityValuesAllowed(arg, value, allowed)
}
_, ok := allowed[value]
normalized := normalizeEntityArgumentValue(arg, value)
_, ok := allowed[normalized]
return ok
}
@ -132,21 +133,43 @@ func entityArgumentAllowedValues(arg *config.ActionArgument) map[string]struct{}
return allowed
}
func checklistEntityValuesAllowed(value string, allowed map[string]struct{}) bool {
parts := strings.Split(value, ",")
sawItem := false
func normalizeEntityArgumentValue(arg *config.ActionArgument, value string) string {
if arg == nil || arg.Entity == "" || len(arg.Choices) != 1 {
return value
}
for _, part := range parts {
part = strings.TrimSpace(part)
if part == "" {
if resolved, ok := entityChoiceValueForTitle(arg, value); ok {
return resolved
}
return value
}
func entityChoiceValueForTitle(arg *config.ActionArgument, title string) (string, bool) {
for _, ent := range entities.GetEntityInstancesOrdered(arg.Entity) {
expandedTitle := tpl.ParseTemplateOfActionBeforeExec(arg.Choices[0].Title, ent)
if title != expandedTitle {
continue
}
sawItem = true
if _, ok := allowed[part]; !ok {
return tpl.ParseTemplateOfActionBeforeExec(arg.Choices[0].Value, ent), true
}
return "", false
}
func checklistEntityValuesAllowed(arg *config.ActionArgument, value string, allowed map[string]struct{}) bool {
segments, err := config.ParseChecklistValue(value)
if err != nil || len(segments) == 0 {
return false
}
for _, segment := range segments {
normalized := normalizeEntityArgumentValue(arg, strings.TrimSpace(segment))
if _, ok := allowed[normalized]; !ok {
return false
}
}
return sawItem
return true
}

53
service/internal/api/api_entity_argument_acl_test.go
View File

@ -128,14 +128,55 @@ func TestStartActionAllowsListedEntityArgumentValue(t *testing.T) {
func TestChecklistEntityValuesAllowedRejectsBlankOnlyInput(t *testing.T) {
allowed := map[string]struct{}{"web01": {}, "db01": {}}
arg := &config.ActionArgument{Type: "checklist"}
assert.False(t, checklistEntityValuesAllowed(",,,", allowed))
assert.False(t, checklistEntityValuesAllowed(" , ", allowed))
assert.False(t, checklistEntityValuesAllowed("", allowed),
assert.False(t, checklistEntityValuesAllowed(arg, ",,,", allowed))
assert.False(t, checklistEntityValuesAllowed(arg, " , ", allowed))
assert.False(t, checklistEntityValuesAllowed(arg, "", allowed),
"all-blank checklist parts are rejected here; empty string is accepted by the caller separately")
assert.True(t, checklistEntityValuesAllowed("web01", allowed))
assert.True(t, checklistEntityValuesAllowed("web01, db01", allowed))
assert.False(t, checklistEntityValuesAllowed("web01, unknown", allowed))
assert.True(t, checklistEntityValuesAllowed(arg, "web01", allowed))
assert.True(t, checklistEntityValuesAllowed(arg, `["web01","db01"]`, allowed))
assert.False(t, checklistEntityValuesAllowed(arg, `["web01","unknown"]`, allowed))
}
func TestChecklistEntityValuesAllowedAcceptsJSONArrayWithEntityTitles(t *testing.T) {
entities.ClearEntitiesOfType("servers")
t.Cleanup(func() {
entities.ClearEntitiesOfType("servers")
})
entities.AddEntity("servers", "0", map[string]any{"name": "web01", "label": "Web Server One"})
entities.AddEntity("servers", "1", map[string]any{"name": "db01", "label": "Database One"})
arg := &config.ActionArgument{
Type: "checklist",
Entity: "servers",
Choices: []config.ActionArgumentChoice{
{Title: "{{ servers.label }}", Value: "{{ servers.name }}"},
},
}
allowed := entityArgumentAllowedValues(arg)
assert.True(t, checklistEntityValuesAllowed(arg, `["Web Server One","Database One"]`, allowed))
assert.False(t, checklistEntityValuesAllowed(arg, `["Web Server One","unknown"]`, allowed))
}
func TestEntityArgumentValueAllowedAcceptsEntityChoiceTitle(t *testing.T) {
entities.ClearEntitiesOfType("servers")
t.Cleanup(func() {
entities.ClearEntitiesOfType("servers")
})
entities.AddEntity("servers", "0", map[string]any{"name": "web01", "label": "Web Server One"})
arg := &config.ActionArgument{
Entity: "servers",
Choices: []config.ActionArgumentChoice{
{Title: "{{ servers.label }}", Value: "{{ servers.name }}"},
},
}
assert.True(t, entityArgumentValueAllowed(arg, "Web Server One"))
assert.True(t, entityArgumentValueAllowed(arg, "web01"))
assert.False(t, entityArgumentValueAllowed(arg, "unknown"))
}
func TestStartActionRejectsMalformedMultiChoiceEntityArgument(t *testing.T) {

12
service/internal/api/api_init_search_hints_test.go
View File

@ -190,9 +190,9 @@ func TestBuildSearchHintsCapsActionsAndEntitiesPerType(t *testing.T) {
entities.ClearEntitiesOfType("cap_container")
})
for i := 0; i < maxSearchHintEntitiesPerType+5; i++ {
entities.AddEntity("cap_host", fmt.Sprintf("%03d", i), map[string]any{"name": fmt.Sprintf("host-%03d", i)})
entities.AddEntity("cap_container", fmt.Sprintf("%03d", i), map[string]any{"name": fmt.Sprintf("ctr-%03d", i)})
for entityIndex := 0; entityIndex < maxSearchHintEntitiesPerType+5; entityIndex++ {
entities.AddEntity("cap_host", fmt.Sprintf("%03d", entityIndex), map[string]any{"name": fmt.Sprintf("host-%03d", entityIndex)})
entities.AddEntity("cap_container", fmt.Sprintf("%03d", entityIndex), map[string]any{"name": fmt.Sprintf("ctr-%03d", entityIndex)})
}
cfg := config.DefaultConfig()
@ -201,10 +201,10 @@ func TestBuildSearchHintsCapsActionsAndEntitiesPerType(t *testing.T) {
{Name: "cap_container", File: "cap_container.yaml"},
}
cfg.Actions = make([]*config.Action, 0, maxSearchHintActions+5)
for i := 0; i < maxSearchHintActions+5; i++ {
for actionIndex := 0; actionIndex < maxSearchHintActions+5; actionIndex++ {
cfg.Actions = append(cfg.Actions, &config.Action{
ID: fmt.Sprintf("action-%03d", i),
Title: fmt.Sprintf("Action %03d", i),
ID: fmt.Sprintf("action-%03d", actionIndex),
Title: fmt.Sprintf("Action %03d", actionIndex),
Shell: "echo",
})
}

90
service/internal/api/api_search_hints.go
View File

@ -27,17 +27,40 @@ func (api *oliveTinAPI) buildSearchHints(user *authpublic.AuthenticatedUser) *ap
}
func (api *oliveTinAPI) buildEntitySearchHints(user *authpublic.AuthenticatedUser) []*apiv1.EntitySearchHint {
hints := entities.ListSearchHints()
out := make([]*apiv1.EntitySearchHint, 0, len(hints))
hintsByType := make(map[string][]*apiv1.EntitySearchHint)
for _, hint := range hints {
for _, hint := range entities.ListSearchHints() {
if allowedHint := api.entitySearchHintIfAllowed(user, hint); allowedHint != nil {
out = append(out, allowedHint)
hintsByType[allowedHint.Type] = appendBoundedEntitySearchHints(
hintsByType[allowedHint.Type],
allowedHint,
maxSearchHintEntitiesPerType,
)
}
}
sortEntitySearchHints(out)
return capEntitySearchHintsPerType(out, maxSearchHintEntitiesPerType)
entityTypes := make([]string, 0, len(hintsByType))
for entityType := range hintsByType {
entityTypes = append(entityTypes, entityType)
}
sort.Strings(entityTypes)
out := make([]*apiv1.EntitySearchHint, 0, len(entityTypes)*maxSearchHintEntitiesPerType)
for _, entityType := range entityTypes {
out = append(out, hintsByType[entityType]...)
}
return out
}
func appendBoundedEntitySearchHints(hints []*apiv1.EntitySearchHint, hint *apiv1.EntitySearchHint, limit int) []*apiv1.EntitySearchHint {
hints = append(hints, hint)
sortEntitySearchHints(hints)
if len(hints) > limit {
hints = hints[:limit]
}
return hints
}
func (api *oliveTinAPI) entitySearchHintIfAllowed(user *authpublic.AuthenticatedUser, hint entities.SearchHint) *apiv1.EntitySearchHint {
@ -70,33 +93,16 @@ func sortEntitySearchHints(hints []*apiv1.EntitySearchHint) {
})
}
func capEntitySearchHintsPerType(hints []*apiv1.EntitySearchHint, perType int) []*apiv1.EntitySearchHint {
if perType < 1 || len(hints) == 0 {
return hints
}
counts := make(map[string]int)
out := make([]*apiv1.EntitySearchHint, 0, len(hints))
for _, hint := range hints {
if counts[hint.Type] >= perType {
continue
}
counts[hint.Type]++
out = append(out, hint)
}
return out
}
func (api *oliveTinAPI) buildActionSearchHints(user *authpublic.AuthenticatedUser) []*apiv1.ActionSearchHint {
candidates := api.collectViewableActionBindings(user)
sortActionSearchCandidates(candidates)
candidates := make([]actionSearchCandidate, 0, maxSearchHintActions)
if len(candidates) > maxSearchHintActions {
candidates = candidates[:maxSearchHintActions]
api.executor.MapActionBindingsLock.RLock()
for _, binding := range api.executor.MapActionBindings {
if candidate, ok := actionSearchCandidateFromBinding(api, user, binding); ok {
candidates = appendBoundedActionSearchCandidates(candidates, candidate, maxSearchHintActions)
}
}
api.executor.MapActionBindingsLock.RUnlock()
out := make([]*apiv1.ActionSearchHint, 0, len(candidates))
for _, candidate := range candidates {
@ -109,26 +115,22 @@ func (api *oliveTinAPI) buildActionSearchHints(user *authpublic.AuthenticatedUse
return out
}
func appendBoundedActionSearchCandidates(candidates []actionSearchCandidate, candidate actionSearchCandidate, limit int) []actionSearchCandidate {
candidates = append(candidates, candidate)
sortActionSearchCandidates(candidates)
if len(candidates) > limit {
candidates = candidates[:limit]
}
return candidates
}
type actionSearchCandidate struct {
title string
bindingID string
hasEntity bool
}
func (api *oliveTinAPI) collectViewableActionBindings(user *authpublic.AuthenticatedUser) []actionSearchCandidate {
api.executor.MapActionBindingsLock.RLock()
defer api.executor.MapActionBindingsLock.RUnlock()
candidates := make([]actionSearchCandidate, 0)
for _, binding := range api.executor.MapActionBindings {
if candidate, ok := actionSearchCandidateFromBinding(api, user, binding); ok {
candidates = append(candidates, candidate)
}
}
return candidates
}
func actionSearchCandidateFromBinding(api *oliveTinAPI, user *authpublic.AuthenticatedUser, binding *executor.ActionBinding) (actionSearchCandidate, bool) {
if !isSearchableActionBinding(binding) {
return actionSearchCandidate{}, false

71
service/internal/api/dashboards.go
View File

@ -8,6 +8,7 @@ import (
acl "github.com/OliveTin/OliveTin/internal/acl"
config "github.com/OliveTin/OliveTin/internal/config"
entities "github.com/OliveTin/OliveTin/internal/entities"
"github.com/OliveTin/OliveTin/internal/executor"
"github.com/OliveTin/OliveTin/internal/tpl"
log "github.com/sirupsen/logrus"
"slices"
@ -135,7 +136,6 @@ func buildDashboardFromConfigWithEntity(dashboard *config.DashboardComponent, rr
}
}
//gocyclo:ignore
func buildDefaultDashboard(rr *DashboardRenderRequest) *apiv1.Dashboard {
db := &apiv1.Dashboard{
Title: "Actions",
@ -149,38 +149,13 @@ func buildDefaultDashboard(rr *DashboardRenderRequest) *apiv1.Dashboard {
}
for _, binding := range rr.ex.MapActionBindings {
if binding == nil || binding.Action == nil || binding.Action.Hidden {
if !defaultBindingEligibleForDashboard(rr, binding) {
continue
}
if binding.IsOnConfiguredDashboard() {
continue
if comp := defaultDashboardComponentFromBinding(binding, rr); comp != nil {
fieldset.Contents = append(fieldset.Contents, comp)
}
if !acl.IsAllowedView(rr.cfg, rr.AuthenticatedUser, binding.Action) {
continue
}
if binding.Entity != nil && binding.Action.Entity != "" &&
!acl.IsAllowedViewEntityType(rr.cfg, rr.AuthenticatedUser, entityFileForType(rr.cfg, binding.Action.Entity)) {
continue
}
action := buildAction(binding, rr)
if action == nil {
continue
}
comp := &apiv1.DashboardComponent{
Type: "link",
Title: action.Title,
Icon: action.Icon,
Action: action,
}
if binding.Entity != nil {
comp.EntityKey = binding.Entity.UniqueKey
}
fieldset.Contents = append(fieldset.Contents, comp)
}
if len(fieldset.Contents) > 0 {
@ -191,6 +166,44 @@ func buildDefaultDashboard(rr *DashboardRenderRequest) *apiv1.Dashboard {
return db
}
func defaultBindingEligibleForDashboard(rr *DashboardRenderRequest, binding *executor.ActionBinding) bool {
return defaultBindingWellFormed(binding) &&
!binding.IsOnConfiguredDashboard() &&
acl.IsAllowedView(rr.cfg, rr.AuthenticatedUser, binding.Action) &&
defaultBindingEntityTypeAllowed(rr, binding)
}
func defaultBindingWellFormed(binding *executor.ActionBinding) bool {
return binding != nil && binding.Action != nil && !binding.Action.Hidden
}
func defaultBindingEntityTypeAllowed(rr *DashboardRenderRequest, binding *executor.ActionBinding) bool {
if binding.Entity == nil || binding.Action.Entity == "" {
return true
}
return acl.IsAllowedViewEntityType(rr.cfg, rr.AuthenticatedUser, entityFileForType(rr.cfg, binding.Action.Entity))
}
func defaultDashboardComponentFromBinding(binding *executor.ActionBinding, rr *DashboardRenderRequest) *apiv1.DashboardComponent {
action := buildAction(binding, rr)
if action == nil {
return nil
}
comp := &apiv1.DashboardComponent{
Type: "link",
Title: action.Title,
Icon: action.Icon,
Action: action,
}
if binding.Entity != nil {
comp.EntityKey = binding.Entity.UniqueKey
}
return comp
}
func entityKeyLess(a, b string) bool {
ai, errA := strconv.ParseInt(a, 10, 64)
bi, errB := strconv.ParseInt(b, 10, 64)

16
service/internal/entities/search_hints_test.go
View File

@ -7,10 +7,10 @@ import (
"github.com/stretchr/testify/require"
)
func TestListSearchHintsReturnsLightweightIdentities(t *testing.T) {
func TestListSearchHintsReturnsLightweightIdentities(testContext *testing.T) {
ClearEntitiesOfType("search_hint_host")
ClearEntitiesOfType("search_hint_app")
t.Cleanup(func() {
testContext.Cleanup(func() {
ClearEntitiesOfType("search_hint_host")
ClearEntitiesOfType("search_hint_app")
})
@ -31,12 +31,12 @@ func TestListSearchHintsReturnsLightweightIdentities(t *testing.T) {
}
host, hostFound := byKey["search_hint_host:0"]
require.True(t, hostFound)
assert.Equal(t, "web01", host.Title)
assert.Equal(t, "search_hint_host", host.Type)
assert.Equal(t, "0", host.UniqueKey)
require.True(testContext, hostFound)
assert.Equal(testContext, "web01", host.Title)
assert.Equal(testContext, "search_hint_host", host.Type)
assert.Equal(testContext, "0", host.UniqueKey)
app, appFound := byKey["search_hint_app:app-1"]
require.True(t, appFound)
assert.Equal(t, "Frontend", app.Title)
require.True(testContext, appFound)
assert.Equal(testContext, "Frontend", app.Title)
}