Merge commit from fork

security: GHSA-jf73-858c-54pg (MODERATE) View permission not being checked when returning dashboards

service/internal/api/api.go

@@ -457,19 +457,38 @@ func (api *oliveTinAPI) GetActionBinding(ctx ctx.Context, req *connect.Request[a
 		return nil, err
 	}
 
-	binding := api.executor.FindBindingByID(req.Msg.BindingId)
-
-	if binding == nil || binding.Action == nil {
-		return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", req.Msg.BindingId))
+	resp, err := api.getActionBindingResponse(user, req.Msg.BindingId)
+	if err != nil {
+		return nil, err
+	}
+	return connect.NewResponse(resp), nil
 }
 
-	return connect.NewResponse(&apiv1.GetActionBindingResponse{
+func (api *oliveTinAPI) getActionBindingResponse(user *authpublic.AuthenticatedUser, bindingId string) (*apiv1.GetActionBindingResponse, error) {
+	binding := api.executor.FindBindingByID(bindingId)
+  
+	if binding == nil || binding.Action == nil {
+		return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", bindingId))
+	}
+  
+	if !api.userCanViewAction(user, binding.Action) {
+		return nil, connect.NewError(connect.CodePermissionDenied, fmt.Errorf("permission denied"))
+	}
+  
+	return &apiv1.GetActionBindingResponse{
 		Action: buildAction(binding, &DashboardRenderRequest{
 			cfg:               api.cfg,
 			AuthenticatedUser: user,
 			ex:                api.executor,
 		}),
-	}), nil
+	}, nil
+}
+
+func (api *oliveTinAPI) userCanViewAction(user *authpublic.AuthenticatedUser, action *config.Action) bool {
+	if user == nil {
+		return true
+	}
+	return acl.IsAllowedView(api.cfg, user, action)
 }
 
 func (api *oliveTinAPI) GetDashboard(ctx ctx.Context, req *connect.Request[apiv1.GetDashboardRequest]) (*connect.Response[apiv1.GetDashboardResponse], error) {

service/internal/api/apiActions.go

@@ -40,6 +40,9 @@ func (rr *DashboardRenderRequest) findActionForEntity(title string, entity *enti
 		if !bindingMatchesTitleAndEntity(binding, title, entity) {
 			continue
 		}
+		if !acl.IsAllowedView(rr.cfg, rr.AuthenticatedUser, binding.Action) {
+			return nil
+		}
 		return buildAction(binding, rr)
 	}
 

service/internal/api/api_test.go

@@ -6,6 +6,7 @@ import (
 
 	"connectrpc.com/connect"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	log "github.com/sirupsen/logrus"
 
@@ -335,3 +336,117 @@ func testWithEntity(t *testing.T, binding *executor.ActionBinding, rr *Dashboard
 	actionResult := buildAction(binding, rr)
 	assert.Equal(t, expectedCanExec, actionResult.CanExec, message)
 }
+
+// buildViewPermissionTestConfig returns config and users for GHSA view-permission tests:
+// one action "secret_action", ACL "restricted" (view:false) for user "low", ACL "full" (view:true) for user "admin".
+func buildViewPermissionTestConfig(t *testing.T) (*config.Config, *authpublic.AuthenticatedUser, *authpublic.AuthenticatedUser) {
+	t.Helper()
+	cfg := config.DefaultConfig()
+	cfg.DefaultPermissions.View = false
+	cfg.DefaultPermissions.Exec = false
+
+	cfg.Actions = append(cfg.Actions, &config.Action{
+		ID:    "secret_action",
+		Title: "Secret Action",
+		Shell: "echo sensitive",
+		Icon:  "🔒",
+	})
+
+	cfg.AccessControlLists = append(cfg.AccessControlLists,
+		&config.AccessControlList{
+			Name:             "restricted",
+			MatchUsernames:   []string{"low"},
+			AddToEveryAction: true,
+			Permissions:      config.PermissionsList{View: false, Exec: false, Logs: false, Kill: false},
+		},
+		&config.AccessControlList{
+			Name:             "full",
+			MatchUsernames:   []string{"admin"},
+			AddToEveryAction: true,
+			Permissions:      config.PermissionsList{View: true, Exec: true, Logs: true, Kill: true},
+		},
+	)
+
+	lowUser := &authpublic.AuthenticatedUser{Username: "low"}
+	lowUser.BuildUserAcls(cfg)
+	adminUser := &authpublic.AuthenticatedUser{Username: "admin"}
+	adminUser.BuildUserAcls(cfg)
+	return cfg, lowUser, adminUser
+}
+
+// TestViewPermissionExcludedFromDashboard (GHSA: view permission) asserts that when a user has view: false,
+// the default dashboard must not include that action. Covers GetDashboard not leaking action metadata.
+func TestViewPermissionExcludedFromDashboard(t *testing.T) {
+	cfg, lowUser, _ := buildViewPermissionTestConfig(t)
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+
+	rr := &DashboardRenderRequest{
+		AuthenticatedUser: lowUser,
+		cfg:               cfg,
+		ex:                ex,
+	}
+	db := buildDefaultDashboard(rr)
+
+	bindingIdsInDashboard := bindingIdsInDashboardContents(db.Contents)
+	assert.NotContains(t, bindingIdsInDashboard, "secret_action",
+		"user with view:false must not see action in dashboard; got bindingIds: %v", bindingIdsInDashboard)
+}
+
+// TestGetActionBindingDeniedWhenNoViewPermission (GHSA: view permission) asserts that GetActionBinding
+// returns permission denied for a user with view: false. Covers GetActionBinding not exposing action details.
+func TestGetActionBindingDeniedWhenNoViewPermission(t *testing.T) {
+	cfg, lowUser, _ := buildViewPermissionTestConfig(t)
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	api := newServer(ex)
+
+	_, err := api.getActionBindingResponse(lowUser, "secret_action")
+	require.Error(t, err)
+	assert.Equal(t, connect.CodePermissionDenied, connect.CodeOf(err),
+		"user with view:false must get permission denied from GetActionBinding")
+}
+
+// TestViewPermissionAllowedSeesAction (GHSA: view permission) asserts that a user with view: true
+// still sees the action in the dashboard and can fetch it via GetActionBinding.
+func TestViewPermissionAllowedSeesAction(t *testing.T) {
+	cfg, _, adminUser := buildViewPermissionTestConfig(t)
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	api := newServer(ex)
+
+	rr := &DashboardRenderRequest{
+		AuthenticatedUser: adminUser,
+		cfg:               cfg,
+		ex:                ex,
+	}
+	db := buildDefaultDashboard(rr)
+	bindingIdsInDashboard := bindingIdsInDashboardContents(db.Contents)
+	assert.Contains(t, bindingIdsInDashboard, "secret_action",
+		"user with view:true must see action in dashboard; got bindingIds: %v", bindingIdsInDashboard)
+
+	resp, err := api.getActionBindingResponse(adminUser, "secret_action")
+	require.NoError(t, err)
+	require.NotNil(t, resp)
+	require.NotNil(t, resp.Action)
+	assert.Equal(t, "secret_action", resp.Action.BindingId)
+}
+
+func bindingIdsInDashboardContents(contents []*apiv1.DashboardComponent) []string {
+	var ids []string
+	for _, c := range contents {
+		ids = append(ids, bindingIdsFromComponent(c)...)
+	}
+	return ids
+}
+
+func bindingIdsFromComponent(c *apiv1.DashboardComponent) []string {
+	if c == nil {
+		return nil
+	}
+	var ids []string
+	if c.Action != nil && c.Action.BindingId != "" {
+		ids = append(ids, c.Action.BindingId)
+	}
+	return append(ids, bindingIdsInDashboardContents(c.Contents)...)
+}

service/internal/api/dashboards.go

@@ -4,6 +4,7 @@ import (
 	"sort"
 
 	apiv1 "github.com/OliveTin/OliveTin/gen/olivetin/api/v1"
+	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/tpl"
@@ -138,6 +139,10 @@ func buildDefaultDashboard(rr *DashboardRenderRequest) *apiv1.Dashboard {
 			continue
 		}
 
+		if !acl.IsAllowedView(rr.cfg, rr.AuthenticatedUser, binding.Action) {
+			continue
+		}
+
 		action := buildAction(binding, rr)
 		if action == nil {
 			continue