From 87928003d8511d4576f663de3f2cdba26e51db65 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky Date: Wed, 26 Jun 2024 08:22:54 -0500 Subject: [PATCH] Only admins and maintainers can run scripts (#20001) #19055 Updated script run permissions -- only admins and maintainers can run arbitrary or saved scripts (not observer or observer+) # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --- changes/19055-scripts-run-permissions | 1 + server/authz/policy.rego | 27 ++------------------------- server/authz/policy_test.go | 12 ++++++------ server/service/scripts.go | 5 ++--- server/service/scripts_test.go | 12 ++++++------ 5 files changed, 17 insertions(+), 40 deletions(-) create mode 100644 changes/19055-scripts-run-permissions diff --git a/changes/19055-scripts-run-permissions b/changes/19055-scripts-run-permissions new file mode 100644 index 0000000000..8c5581d893 --- /dev/null +++ b/changes/19055-scripts-run-permissions @@ -0,0 +1 @@ +- Updated script run permissions -- only admins and maintainers can run arbitrary or saved scripts (not observer or observer+) diff --git a/server/authz/policy.rego b/server/authz/policy.rego index 8f9053472c..639b1510ff 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -929,25 +929,14 @@ allow { # Host Script Result (script execution and output) ## -# Global admins and maintainers can write (execute) anonymous scripts (not +# Global admins and maintainers can write (execute) scripts (not # gitops as this is not something that relates to fleetctl apply). allow { object.type == "host_script_result" - is_null(object.script_id) subject.global_role == [admin, maintainer][_] action == write } -# Global admins, maintainers, observer_plus and observers can write (execute) -# saved scripts (not gitops as this is not something that relates to fleetctl -# apply). -allow { - object.type == "host_script_result" - not is_null(object.script_id) - subject.global_role == [admin, maintainer, observer, observer_plus][_] - action == write -} - # Global admins, maintainers, observer_plus and observers can read scripts. allow { object.type == "host_script_result" @@ -955,27 +944,15 @@ allow { action == read } -# Team admin and maintainers can write (execute) anonymous scripts for their +# Team admin and maintainers can write (execute) scripts for their # teams (not gitops as this is not something that relates to fleetctl apply). allow { object.type == "host_script_result" - is_null(object.script_id) not is_null(object.team_id) team_role(subject, object.team_id) == [admin, maintainer][_] action == write } -# Team admins, maintainers, observer_plus and observers can write (execute) -# saved scripts for their teams (not gitops as this is not something that -# relates to fleetctl apply). -allow { - object.type == "host_script_result" - not is_null(object.script_id) - not is_null(object.team_id) - team_role(subject, object.team_id) == [admin, maintainer, observer_plus, observer][_] - action == write -} - # Team admins, maintainers, observer_plus and observers can read scripts for their teams. allow { object.type == "host_script_result" diff --git a/server/authz/policy_test.go b/server/authz/policy_test.go index 2ab7980d13..27d682ac77 100644 --- a/server/authz/policy_test.go +++ b/server/authz/policy_test.go @@ -2101,20 +2101,20 @@ func TestAuthorizeHostScriptResult(t *testing.T) { {user: test.UserObserver, object: globalScript, action: write, allow: false}, {user: test.UserObserver, object: globalScript, action: read, allow: true}, - {user: test.UserObserver, object: globalSavedScript, action: write, allow: true}, + {user: test.UserObserver, object: globalSavedScript, action: write, allow: false}, {user: test.UserObserver, object: globalSavedScript, action: read, allow: true}, {user: test.UserObserver, object: team1Script, action: write, allow: false}, {user: test.UserObserver, object: team1Script, action: read, allow: true}, - {user: test.UserObserver, object: team1SavedScript, action: write, allow: true}, + {user: test.UserObserver, object: team1SavedScript, action: write, allow: false}, {user: test.UserObserver, object: team1SavedScript, action: read, allow: true}, {user: test.UserObserverPlus, object: globalScript, action: write, allow: false}, {user: test.UserObserverPlus, object: globalScript, action: read, allow: true}, - {user: test.UserObserverPlus, object: globalSavedScript, action: write, allow: true}, + {user: test.UserObserverPlus, object: globalSavedScript, action: write, allow: false}, {user: test.UserObserverPlus, object: globalSavedScript, action: read, allow: true}, {user: test.UserObserverPlus, object: team1Script, action: write, allow: false}, {user: test.UserObserverPlus, object: team1Script, action: read, allow: true}, - {user: test.UserObserverPlus, object: team1SavedScript, action: write, allow: true}, + {user: test.UserObserverPlus, object: team1SavedScript, action: write, allow: false}, {user: test.UserObserverPlus, object: team1SavedScript, action: read, allow: true}, {user: test.UserGitOps, object: globalScript, action: write, allow: false}, @@ -2168,7 +2168,7 @@ func TestAuthorizeHostScriptResult(t *testing.T) { {user: test.UserTeamObserverTeam1, object: globalSavedScript, action: read, allow: false}, {user: test.UserTeamObserverTeam1, object: team1Script, action: write, allow: false}, {user: test.UserTeamObserverTeam1, object: team1Script, action: read, allow: true}, - {user: test.UserTeamObserverTeam1, object: team1SavedScript, action: write, allow: true}, + {user: test.UserTeamObserverTeam1, object: team1SavedScript, action: write, allow: false}, {user: test.UserTeamObserverTeam1, object: team1SavedScript, action: read, allow: true}, {user: test.UserTeamObserverTeam2, object: globalScript, action: write, allow: false}, @@ -2186,7 +2186,7 @@ func TestAuthorizeHostScriptResult(t *testing.T) { {user: test.UserTeamObserverPlusTeam1, object: globalSavedScript, action: read, allow: false}, {user: test.UserTeamObserverPlusTeam1, object: team1Script, action: write, allow: false}, {user: test.UserTeamObserverPlusTeam1, object: team1Script, action: read, allow: true}, - {user: test.UserTeamObserverPlusTeam1, object: team1SavedScript, action: write, allow: true}, + {user: test.UserTeamObserverPlusTeam1, object: team1SavedScript, action: write, allow: false}, {user: test.UserTeamObserverPlusTeam1, object: team1SavedScript, action: read, allow: true}, {user: test.UserTeamObserverPlusTeam2, object: globalScript, action: write, allow: false}, diff --git a/server/service/scripts.go b/server/service/scripts.go index fb847352b4..91392c6d88 100644 --- a/server/service/scripts.go +++ b/server/service/scripts.go @@ -211,9 +211,8 @@ func (svc *Service) RunHostScript(ctx context.Context, request *fleet.HostScript maxPending := maxPendingScripts - // authorize with the host's team and the script id provided, as both affect - // the permissions. - if err := svc.authz.Authorize(ctx, &fleet.HostScriptResult{TeamID: host.TeamID, ScriptID: request.ScriptID}, fleet.ActionWrite); err != nil { + // authorize with the host's team + if err := svc.authz.Authorize(ctx, &fleet.HostScriptResult{TeamID: host.TeamID}, fleet.ActionWrite); err != nil { return nil, err } diff --git a/server/service/scripts_test.go b/server/service/scripts_test.go index b12e4cf5a6..ffb3909882 100644 --- a/server/service/scripts_test.go +++ b/server/service/scripts_test.go @@ -105,8 +105,8 @@ func TestHostRunScript(t *testing.T) { name: "global observer saved", user: &fleet.User{GlobalRole: ptr.String(fleet.RoleObserver)}, scriptID: ptr.Uint(1), - shouldFailTeamWrite: false, - shouldFailGlobalWrite: false, + shouldFailTeamWrite: true, + shouldFailGlobalWrite: true, }, { name: "global observer+", @@ -118,8 +118,8 @@ func TestHostRunScript(t *testing.T) { name: "global observer+ saved", user: &fleet.User{GlobalRole: ptr.String(fleet.RoleObserverPlus)}, scriptID: ptr.Uint(1), - shouldFailTeamWrite: false, - shouldFailGlobalWrite: false, + shouldFailTeamWrite: true, + shouldFailGlobalWrite: true, }, { name: "global gitops", @@ -170,7 +170,7 @@ func TestHostRunScript(t *testing.T) { name: "team observer, belongs to team, saved", user: &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleObserver}}}, scriptID: ptr.Uint(1), - shouldFailTeamWrite: false, + shouldFailTeamWrite: true, shouldFailGlobalWrite: true, }, { @@ -183,7 +183,7 @@ func TestHostRunScript(t *testing.T) { name: "team observer+, belongs to team, saved", user: &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleObserverPlus}}}, scriptID: ptr.Uint(1), - shouldFailTeamWrite: false, + shouldFailTeamWrite: true, shouldFailGlobalWrite: true, }, {