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

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

This commit is contained in:
jamesread 2026-03-05 08:20:02 +00:00
parent 235493e471
commit d7962710e7
4 changed files with 272 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

163
service/internal/api/api.go
View File

@ -70,20 +70,21 @@ func (api *oliveTinAPI) KillAction(ctx ctx.Context, req *connect.Request[apiv1.K
execReqLogEntry, ret.Found = api.executor.GetLog(req.Msg.ExecutionTrackingId)
if !ret.Found {
log.Warnf("Killing execution request not possible - not found by tracking ID: %v", req.Msg.ExecutionTrackingId)
return connect.NewResponse(ret), nil
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found for tracking ID %s", req.Msg.ExecutionTrackingId))
}
log.Warnf("Killing execution request by tracking ID: %v", req.Msg.ExecutionTrackingId)
if execReqLogEntry.Binding == nil {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("log entry has no binding for tracking ID %s", req.Msg.ExecutionTrackingId))
}
action := execReqLogEntry.Binding.Action
if action == nil {
log.Warnf("Killing execution request not possible - action not found: %v", execReqLogEntry.ActionTitle)
ret.Killed = false
return connect.NewResponse(ret), nil
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action not found for tracking ID %s", req.Msg.ExecutionTrackingId))
}
log.Warnf("Killing execution request by tracking ID: %v", req.Msg.ExecutionTrackingId)
user := auth.UserFromApiCall(ctx, req, api.cfg)
api.killActionByTrackingId(user, action, execReqLogEntry, ret)
@ -205,42 +206,58 @@ func (api *oliveTinAPI) LocalUserLogin(ctx ctx.Context, req *connect.Request[api
return response, nil
}
func (api *oliveTinAPI) StartActionAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionAndWaitRequest]) (*connect.Response[apiv1.StartActionAndWaitResponse], error) {
args := make(map[string]string)
for _, arg := range req.Msg.Arguments {
args[arg.Name] = arg.Value
}
user := auth.UserFromApiCall(ctx, req, api.cfg)
func (api *oliveTinAPI) startActionAndWaitRun(binding *executor.ActionBinding, args map[string]string, user *authpublic.AuthenticatedUser) (*executor.InternalLogEntry, bool) {
execReq := executor.ExecutionRequest{
Binding: api.executor.FindBindingByID(req.Msg.ActionId),
Binding: binding,
TrackingID: uuid.NewString(),
Arguments: args,
AuthenticatedUser: user,
Cfg: api.cfg,
}
wg, _ := api.executor.ExecRequest(&execReq)
wg.Wait()
return api.executor.GetLog(execReq.TrackingID)
}
internalLogEntry, ok := api.executor.GetLog(execReq.TrackingID)
if ok {
return connect.NewResponse(&apiv1.StartActionAndWaitResponse{
LogEntry: api.internalLogEntryToPb(internalLogEntry, user),
}), nil
} else {
return nil, fmt.Errorf("execution not found")
func (api *oliveTinAPI) findBindingOrNotFound(actionId string) (*executor.ActionBinding, error) {
binding := api.executor.FindBindingByID(actionId)
if binding == nil || binding.Action == nil {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", actionId))
}
return binding, nil
}
func (api *oliveTinAPI) StartActionAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionAndWaitRequest]) (*connect.Response[apiv1.StartActionAndWaitResponse], error) {
binding, err := api.findBindingOrNotFound(req.Msg.ActionId)
if err != nil {
return nil, err
}
args := make(map[string]string)
for _, arg := range req.Msg.Arguments {
args[arg.Name] = arg.Value
}
user := auth.UserFromApiCall(ctx, req, api.cfg)
internalLogEntry, ok := api.startActionAndWaitRun(binding, args, user)
if !ok {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found"))
}
return connect.NewResponse(&apiv1.StartActionAndWaitResponse{
LogEntry: api.internalLogEntryToPb(internalLogEntry, user),
}), nil
}
func (api *oliveTinAPI) StartActionByGet(ctx ctx.Context, req *connect.Request[apiv1.StartActionByGetRequest]) (*connect.Response[apiv1.StartActionByGetResponse], error) {
binding := api.executor.FindBindingByID(req.Msg.ActionId)
if binding == nil || binding.Action == nil {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", req.Msg.ActionId))
}
args := make(map[string]string)
execReq := executor.ExecutionRequest{
Binding: api.executor.FindBindingByID(req.Msg.ActionId),
Binding: binding,
TrackingID: uuid.NewString(),
Arguments: args,
AuthenticatedUser: auth.UserFromApiCall(ctx, req, api.cfg),
@ -255,12 +272,17 @@ func (api *oliveTinAPI) StartActionByGet(ctx ctx.Context, req *connect.Request[a
}
func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionByGetAndWaitRequest]) (*connect.Response[apiv1.StartActionByGetAndWaitResponse], error) {
binding := api.executor.FindBindingByID(req.Msg.ActionId)
if binding == nil || binding.Action == nil {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", req.Msg.ActionId))
}
args := make(map[string]string)
user := auth.UserFromApiCall(ctx, req, api.cfg)
execReq := executor.ExecutionRequest{
Binding: api.executor.FindBindingByID(req.Msg.ActionId),
Binding: binding,
TrackingID: uuid.NewString(),
Arguments: args,
AuthenticatedUser: user,
@ -276,9 +298,8 @@ func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Re
return connect.NewResponse(&apiv1.StartActionByGetAndWaitResponse{
LogEntry: api.internalLogEntryToPb(internalLogEntry, user),
}), nil
} else {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found"))
}
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found"))
}
func calculateRateLimitExpires(api *oliveTinAPI, logEntry *executor.InternalLogEntry) string {
@ -392,6 +413,8 @@ func (api *oliveTinAPI) ExecutionStatus(ctx ctx.Context, req *connect.Request[ap
func (api *oliveTinAPI) Logout(ctx ctx.Context, req *connect.Request[apiv1.LogoutRequest]) (*connect.Response[apiv1.LogoutResponse], error) {
user := auth.UserFromApiCall(ctx, req, api.cfg)
auth.RevokeSessionForProvider(api.cfg, user.Provider, user.SID)
log.WithFields(log.Fields{
"username": user.Username,
"provider": user.Provider,
@ -434,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 {
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 {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", bindingId))
}
if 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) {
@ -646,7 +688,16 @@ error messages more quickly before starting the action.
It uses the same validation logic as the executor, including mangling argument
values (e.g., datetime formatting, checkbox title-to-value conversion).
*/
func (api *oliveTinAPI) argumentNotFoundForValidation(msg *apiv1.ValidateArgumentTypeRequest) bool {
arg, _ := api.findArgumentForValidation(msg.BindingId, msg.ArgumentName)
return arg == nil && (msg.BindingId != "" || msg.ArgumentName != "")
}
func (api *oliveTinAPI) ValidateArgumentType(ctx ctx.Context, req *connect.Request[apiv1.ValidateArgumentTypeRequest]) (*connect.Response[apiv1.ValidateArgumentTypeResponse], error) {
if api.argumentNotFoundForValidation(req.Msg) {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action or argument not found for binding ID %s", req.Msg.BindingId))
}
err := api.validateArgumentTypeInternal(req.Msg)
desc := ""
if err != nil {
@ -747,6 +798,13 @@ func (api *oliveTinAPI) DumpVars(ctx ctx.Context, req *connect.Request[apiv1.Dum
return connect.NewResponse(res), nil
}
func debugBindingActionTitle(binding *executor.ActionBinding) string {
if binding == nil || binding.Action == nil {
return ""
}
return binding.Action.Title
}
func (api *oliveTinAPI) DumpPublicIdActionMap(ctx ctx.Context, req *connect.Request[apiv1.DumpPublicIdActionMapRequest]) (*connect.Response[apiv1.DumpPublicIdActionMapResponse], error) {
res := &apiv1.DumpPublicIdActionMapResponse{}
res.Contents = make(map[string]*apiv1.DebugBinding)
@ -761,7 +819,7 @@ func (api *oliveTinAPI) DumpPublicIdActionMap(ctx ctx.Context, req *connect.Requ
for k, v := range api.executor.MapActionBindings {
res.Contents[k] = &apiv1.DebugBinding{
ActionTitle: v.Action.Title,
ActionTitle: debugBindingActionTitle(v),
}
}
@ -1271,30 +1329,37 @@ func (api *oliveTinAPI) RestartAction(ctx ctx.Context, req *connect.Request[apiv
ExecutionTrackingId: req.Msg.ExecutionTrackingId,
}
var execReqLogEntry *executor.InternalLogEntry
execReqLogEntry, found := api.executor.GetLog(req.Msg.ExecutionTrackingId)
if !found {
log.Warnf("Restarting execution request not possible - not found by tracking ID: %v", req.Msg.ExecutionTrackingId)
return connect.NewResponse(ret), nil
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found for tracking ID %s", req.Msg.ExecutionTrackingId))
}
log.Warnf("Restarting execution request by tracking ID: %v", req.Msg.ExecutionTrackingId)
if execReqLogEntry.Binding == nil {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("log entry has no binding for tracking ID %s", req.Msg.ExecutionTrackingId))
}
action := execReqLogEntry.Binding.Action
if action == nil {
log.Warnf("Restarting execution request not possible - action not found: %v", execReqLogEntry.ActionTitle)
return connect.NewResponse(ret), nil
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action not found for tracking ID %s", req.Msg.ExecutionTrackingId))
}
return api.StartAction(ctx, &connect.Request[apiv1.StartActionRequest]{
Msg: &apiv1.StartActionRequest{
BindingId: execReqLogEntry.GetBindingId(),
UniqueTrackingId: req.Msg.ExecutionTrackingId,
},
})
authenticatedUser := auth.UserFromApiCall(ctx, req, api.cfg)
// TrackingID is deliberately not passed to the executor, so that it generates a new one for the restarted execution.
// This is because the old execution (identified by the old TrackingID) is already used.
execReq := executor.ExecutionRequest{
Binding: execReqLogEntry.Binding,
Arguments: make(map[string]string),
AuthenticatedUser: authenticatedUser,
Cfg: api.cfg,
}
api.executor.ExecRequest(&execReq)
ret.ExecutionTrackingId = execReq.TrackingID
return connect.NewResponse(ret), nil
}
func newServer(ex *executor.Executor) *oliveTinAPI {

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

@ -28,18 +28,22 @@ func (rr *DashboardRenderRequest) findAction(title string) *apiv1.Action {
return rr.findActionForEntity(title, nil)
}
func bindingMatchesTitleAndEntity(binding *executor.ActionBinding, title string, entity *entities.Entity) bool {
return binding != nil && binding.Action != nil && binding.Action.Title == title && matchesEntity(binding, entity)
}
func (rr *DashboardRenderRequest) findActionForEntity(title string, entity *entities.Entity) *apiv1.Action {
rr.ex.MapActionBindingsLock.RLock()
defer rr.ex.MapActionBindingsLock.RUnlock()
for _, binding := range rr.ex.MapActionBindings {
if binding.Action.Title != title {
if !bindingMatchesTitleAndEntity(binding, title, entity) {
continue
}
if matchesEntity(binding, entity) {
return buildAction(binding, rr)
if !acl.IsAllowedView(rr.cfg, rr.AuthenticatedUser, binding.Action) {
return nil
}
return buildAction(binding, rr)
}
return nil
@ -117,26 +121,37 @@ func getDefaultArgumentValue(cfgArg config.ActionArgument, entity *entities.Enti
return defaultValue
}
func formatRateLimitExpiry(expiryUnix int64) string {
if expiryUnix <= 0 {
return ""
}
return time.Unix(expiryUnix, 0).Format("2006-01-02 15:04:05")
}
func actionFromBinding(actionBinding *executor.ActionBinding) (*executor.ActionBinding, *config.Action) {
if actionBinding == nil || actionBinding.Action == nil {
return nil, nil
}
return actionBinding, actionBinding.Action
}
func buildAction(actionBinding *executor.ActionBinding, rr *DashboardRenderRequest) *apiv1.Action {
action := actionBinding.Action
aclCanExec := acl.IsAllowedExec(rr.cfg, rr.AuthenticatedUser, action)
enabledExprCanExec := evaluateEnabledExpression(action, actionBinding.Entity)
// Calculate rate limit expiry time
expiryUnix := rr.ex.GetTimeUntilAvailable(actionBinding)
datetimeRateLimitExpires := ""
if expiryUnix > 0 {
datetimeRateLimitExpires = time.Unix(expiryUnix, 0).Format("2006-01-02 15:04:05")
binding, action := actionFromBinding(actionBinding)
if binding == nil {
return nil
}
aclCanExec := acl.IsAllowedExec(rr.cfg, rr.AuthenticatedUser, action)
enabledExprCanExec := evaluateEnabledExpression(action, binding.Entity)
datetimeRateLimitExpires := formatRateLimitExpiry(rr.ex.GetTimeUntilAvailable(binding))
btn := apiv1.Action{
BindingId: actionBinding.ID,
Title: tpl.ParseTemplateOfActionBeforeExec(action.Title, actionBinding.Entity),
Icon: tpl.ParseTemplateOfActionBeforeExec(action.Icon, actionBinding.Entity),
BindingId: binding.ID,
Title: tpl.ParseTemplateOfActionBeforeExec(action.Title, binding.Entity),
Icon: tpl.ParseTemplateOfActionBeforeExec(action.Icon, binding.Entity),
CanExec: aclCanExec && enabledExprCanExec,
PopupOnStart: action.PopupOnStart,
Order: int32(actionBinding.ConfigOrder),
Order: int32(binding.ConfigOrder),
Timeout: int32(action.Timeout),
DatetimeRateLimitExpires: datetimeRateLimitExpires,
}
@ -147,7 +162,7 @@ func buildAction(actionBinding *executor.ActionBinding, rr *DashboardRenderReque
Title: cfgArg.Title,
Type: cfgArg.Type,
Description: cfgArg.Description,
DefaultValue: getDefaultArgumentValue(cfgArg, actionBinding.Entity),
DefaultValue: getDefaultArgumentValue(cfgArg, binding.Entity),
Choices: buildChoices(cfgArg),
Suggestions: cfgArg.Suggestions,
SuggestionsBrowserKey: cfgArg.SuggestionsBrowserKey,

115
service/internal/api/api_test.go
View File

@ -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)...)
}

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

@ -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"
@ -130,7 +131,7 @@ func buildDefaultDashboard(rr *DashboardRenderRequest) *apiv1.Dashboard {
}
for _, binding := range rr.ex.MapActionBindings {
if binding.Action.Hidden {
if binding == nil || binding.Action == nil || binding.Action.Hidden {
continue
}
@ -138,7 +139,14 @@ 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
}
fieldset.Contents = append(fieldset.Contents, &apiv1.DashboardComponent{
Type: "link",