Просмотр исходного кода

fix: Hidden acctions are for display purposes only, not security

jamesread 1 неделя назад
Родитель
Сommit
1303a27d05

+ 1 - 0
docs/modules/ROOT/nav.adoc

@@ -57,6 +57,7 @@
 ** xref:action_customization/ratelimiting.adoc[Rate Limiting]
 ** xref:action_customization/enabledExpression.adoc[Enabled Expression]
 ** xref:action_customization/ids.adoc[IDs]
+** xref:action_customization/hidden.adoc[Hidden]
 * xref:action_examples/intro.adoc[Action Examples]
 ** xref:action_examples/containers.adoc[Containers - start/stop]
 *** xref:action_examples/docker-proxy.adoc[Docker control, via proxy]

+ 17 - 0
docs/modules/ROOT/pages/action_customization/hidden.adoc

@@ -0,0 +1,17 @@
+[#action-hidden]
+= Hidden actions
+
+Set `hidden: true` on an action to keep it off the default **Actions** dashboard and out of automatic UI listings (for example entity-related action pickers). This is useful for background helpers that run on cron, startup, webhooks, or as xref:action_execution/triggers.adoc[triggers].
+
+[source,yaml]
+----
+actions:
+  - title: Update container entity file
+    shell: docker ps -a --format=json > /etc/OliveTin/entities/containers.json
+    hidden: true
+    execOnStartup: true
+----
+
+IMPORTANT: `hidden` is **not** a security control. It only affects where the action appears in the UI. Users who are allowed to view the action (via `defaultPermissions` or ACLs) can still open its Action Details page, see it in logs, and call APIs such as `GetActionBinding`. To restrict who can see or run an action, use xref:security/acl.adoc[Access Control Lists].
+
+You can still place a hidden action on a custom dashboard by title if you want a button for it in a specific place.

+ 2 - 0
docs/modules/ROOT/pages/action_customization/intro.adoc

@@ -12,6 +12,7 @@ You can customize actions by:
 * Setting rate limits to prevent actions from being executed too frequently
 * Using enabled expressions to dynamically enable/disable actions based on entity state
 * Assigning unique IDs for API access
+* Hiding background helpers from the default dashboard
 * Configuring log saving for audit trails
 
 See the links in this section for detailed information on each customization option.
@@ -28,3 +29,4 @@ Explore specific customization options:
 * xref:action_customization/enabledExpression.adoc[Enabled expressions] - Dynamically enable/disable actions based on entity state
 * xref:logs/saving.adoc[Save action logs] - Configure log retention for actions
 * xref:action_customization/ids.adoc[Set action IDs] - Assign IDs for API access
+* xref:action_customization/hidden.adoc[Hidden actions] - Keep background helpers off the default dashboard (not a security control)

+ 2 - 1
docs/modules/ROOT/pages/action_execution/triggers.adoc

@@ -1,7 +1,7 @@
 [#triggers]
 = Triggers
 
-Sometimes you want to trigger another action after the first one completes. This is mostly useful for updating hidden actions that update entity files, without having to run those updates on a cron job every 10 seconds!
+Sometimes you want to trigger another action after the first one completes. This is mostly useful for updating xref:action_customization/hidden.adoc[hidden] actions that update entity files, without having to run those updates on a cron job every 10 seconds!
 
 NOTE: OliveTin used to support a single action trigger, but now supports multiple triggers. The field `trigger` was renamed to `triggers` and is now an array of triggers.
 
@@ -23,3 +23,4 @@ actions:
     hidden: true
 ----
 
+IMPORTANT: `hidden` only removes the action from automatic dashboard listings. It is not a security control — use xref:security/acl.adoc[ACLs] to restrict who can view or execute an action. See xref:action_customization/hidden.adoc[Hidden actions].

+ 2 - 0
docs/modules/ROOT/pages/security/acl.adoc

@@ -158,6 +158,8 @@ dashboards:
 
 In the example above, guests can open **Public tools**, but **Services** is hidden from the side menu and cannot be loaded by deep link.
 
+NOTE: Action `hidden: true` is not part of the ACL model. It only controls dashboard listing. Restrict who can see or run actions with the permissions above; see xref:action_customization/hidden.adoc[Hidden actions].
+
 == ACL Matching - usernames and usergroups.
 
 You can match users based on their usergroup which is the most common, but it is also possible to match based on the user's username.

+ 2 - 5
service/internal/acl/acl.go

@@ -119,12 +119,9 @@ func IsAllowedExec(cfg *config.Config, user *authpublic.AuthenticatedUser, actio
 	return aclCheck(Exec, cfg.DefaultPermissions.Exec, cfg, "isAllowedExec", user, action.Title, action.Acls, true)
 }
 
-// IsAllowedView checks if a User is allowed to view an Action
+// IsAllowedView checks if a User is allowed to view an Action.
+// Action.Hidden is not a security control — it only affects dashboard listing.
 func IsAllowedView(cfg *config.Config, user *authpublic.AuthenticatedUser, action *config.Action) bool {
-	if action.Hidden {
-		return false
-	}
-
 	return aclCheck(View, cfg.DefaultPermissions.View, cfg, "isAllowedView", user, action.Title, action.Acls, true)
 }
 

+ 46 - 0
service/internal/acl/acl_hidden_test.go

@@ -0,0 +1,46 @@
+package acl
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	authpublic "github.com/OliveTin/OliveTin/internal/auth/authpublic"
+	config "github.com/OliveTin/OliveTin/internal/config"
+)
+
+func TestIsAllowedViewIgnoresHiddenFlag(t *testing.T) {
+	cfg := config.DefaultConfig()
+	guest := &authpublic.AuthenticatedUser{Username: "guest", Provider: "system"}
+	guest.BuildUserAcls(cfg)
+
+	visible := &config.Action{Title: "Visible", Shell: "echo"}
+	hidden := &config.Action{Title: "Hidden", Shell: "echo", Hidden: true}
+
+	assert.True(t, IsAllowedView(cfg, guest, visible))
+	assert.True(t, IsAllowedView(cfg, guest, hidden),
+		"hidden must not deny view; use ACLs to restrict access")
+}
+
+func TestIsAllowedViewHiddenStillRespectsAcl(t *testing.T) {
+	cfg := config.DefaultConfig()
+	cfg.DefaultPermissions.View = false
+	cfg.AccessControlLists = []*config.AccessControlList{
+		{
+			Name:             "admins",
+			MatchUsernames:   []string{"admin"},
+			AddToEveryAction: true,
+			Permissions:      config.PermissionsList{View: true},
+		},
+	}
+
+	admin := &authpublic.AuthenticatedUser{Username: "admin"}
+	admin.BuildUserAcls(cfg)
+	guest := &authpublic.AuthenticatedUser{Username: "guest", Provider: "system"}
+	guest.BuildUserAcls(cfg)
+
+	hidden := &config.Action{Title: "Webhook", Shell: "echo", Hidden: true}
+
+	assert.True(t, IsAllowedView(cfg, admin, hidden))
+	assert.False(t, IsAllowedView(cfg, guest, hidden))
+}

+ 5 - 1
service/internal/api/api_entity_related_actions.go

@@ -54,7 +54,11 @@ func tryAppendRelatedCandidate(candidates *[]relatedActionCandidate, seen map[st
 }
 
 func bindingViewableForRelated(seen map[string]bool, api *oliveTinAPI, user *authpublic.AuthenticatedUser, binding *executor.ActionBinding) bool {
-	return binding != nil && binding.Action != nil && !seen[binding.ID] && api.userCanViewAction(user, binding.Action)
+	if binding == nil || binding.Action == nil || seen[binding.ID] || binding.Action.Hidden {
+		return false
+	}
+
+	return api.userCanViewAction(user, binding.Action)
 }
 
 func relatedPrefillForBinding(binding *executor.ActionBinding, entityType string, entity *entities.Entity) (map[string]string, bool) {

+ 36 - 0
service/internal/api/api_test.go

@@ -632,6 +632,42 @@ func TestViewPermissionAllowedSeesAction(t *testing.T) {
 	assert.Equal(t, "secret_action", resp.Action.BindingId)
 }
 
+// TestGetActionBindingAllowsHiddenWhenViewAllowed asserts that hidden is not a security control:
+// users with view permission can fetch action details for hidden actions (e.g. webhook helpers).
+func TestGetActionBindingAllowsHiddenWhenViewAllowed(t *testing.T) {
+	cfg, lowUser, adminUser := buildViewPermissionTestConfig(t)
+	cfg.Actions = append(cfg.Actions, &config.Action{
+		ID:     "webhook_helper",
+		Title:  "Webhook Helper",
+		Shell:  "echo webhook",
+		Hidden: true,
+	})
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	api := newServer(ex)
+
+	rr := &DashboardRenderRequest{
+		AuthenticatedUser: adminUser,
+		cfg:               cfg,
+		ex:                ex,
+	}
+	db := buildDefaultDashboard(rr)
+	assert.NotContains(t, bindingIdsInDashboardContents(db.Contents), "webhook_helper",
+		"hidden actions must stay off the default dashboard")
+
+	resp, err := api.getActionBindingResponse(adminUser, "webhook_helper")
+	require.NoError(t, err)
+	require.NotNil(t, resp)
+	require.NotNil(t, resp.Action)
+	assert.Equal(t, "webhook_helper", resp.Action.BindingId)
+	assert.Equal(t, "Webhook Helper", resp.Action.Title)
+
+	_, err = api.getActionBindingResponse(lowUser, "webhook_helper")
+	require.Error(t, err)
+	assert.Equal(t, connect.CodePermissionDenied, connect.CodeOf(err),
+		"users without view ACL must still be denied for hidden actions")
+}
+
 // 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) {