From b524da11e9466d05fe03304713ee1c61bb276ec4 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 10 Nov 2024 11:52:31 +0100 Subject: [PATCH] EventManager: disable commands by default Signed-off-by: Nicola Murino --- internal/common/protocol_test.go | 25 +++++++++++++++++++++---- internal/dataprovider/eventrule.go | 5 +---- internal/httpd/httpd_test.go | 19 +++++++++++++++++++ templates/webadmin/eventaction.html | 15 ++++++--------- 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/internal/common/protocol_test.go b/internal/common/protocol_test.go index 0aa56853..bce64515 100644 --- a/internal/common/protocol_test.go +++ b/internal/common/protocol_test.go @@ -3928,6 +3928,11 @@ func TestEventRule(t *testing.T) { err = os.WriteFile(uploadScriptPath, getUploadScriptContent(movedPath, "", 0), 0755) assert.NoError(t, err) + dataprovider.EnabledActionCommands = []string{uploadScriptPath} + defer func() { + dataprovider.EnabledActionCommands = nil + }() + action1.Type = dataprovider.ActionTypeCommand action1.Options = dataprovider.BaseEventActionOptions{ CmdConfig: dataprovider.EventActionCommandConfig{ @@ -4265,6 +4270,10 @@ func TestEventRuleDisabledCommand(t *testing.T) { }, }, } + _, _, err = httpdtest.AddEventAction(a1, http.StatusBadRequest) + assert.NoError(t, err) + // Enable the command to allow saving + dataprovider.EnabledActionCommands = []string{a1.Options.CmdConfig.Cmd} action1, _, err := httpdtest.AddEventAction(a1, http.StatusCreated) assert.NoError(t, err) action2, _, err := httpdtest.AddEventAction(a2, http.StatusCreated) @@ -4312,8 +4321,8 @@ func TestEventRuleDisabledCommand(t *testing.T) { } rule, _, err := httpdtest.AddEventRule(r, http.StatusCreated) assert.NoError(t, err) - // restrit command execution - dataprovider.EnabledActionCommands = []string{"/bin/ls"} + // restrict command execution + dataprovider.EnabledActionCommands = nil lastReceivedEmail.reset() // create a folder to trigger the rule @@ -4335,8 +4344,6 @@ func TestEventRuleDisabledCommand(t *testing.T) { assert.Contains(t, email.Data, fmt.Sprintf("Object name: %s object type: folder", folder.Name)) lastReceivedEmail.reset() - dataprovider.EnabledActionCommands = nil - _, err = httpdtest.RemoveFolder(folder, http.StatusOK) assert.NoError(t, err) @@ -4368,6 +4375,11 @@ func TestEventRuleProviderEvents(t *testing.T) { err = os.WriteFile(saveObjectScriptPath, getSaveProviderObjectScriptContent(outPath, 0), 0755) assert.NoError(t, err) + dataprovider.EnabledActionCommands = []string{saveObjectScriptPath} + defer func() { + dataprovider.EnabledActionCommands = nil + }() + a1 := dataprovider.BaseEventAction{ Name: "a1", Type: dataprovider.ActionTypeCommand, @@ -5231,6 +5243,11 @@ func TestEventActionCommandEnvVars(t *testing.T) { envName := "MY_ENV" uploadScriptPath := filepath.Join(os.TempDir(), "upload.sh") + dataprovider.EnabledActionCommands = []string{uploadScriptPath} + defer func() { + dataprovider.EnabledActionCommands = nil + }() + err := os.WriteFile(uploadScriptPath, getUploadScriptEnvContent(envName), 0755) assert.NoError(t, err) a1 := dataprovider.BaseEventAction{ diff --git a/internal/dataprovider/eventrule.go b/internal/dataprovider/eventrule.go index e2309d69..82f80746 100644 --- a/internal/dataprovider/eventrule.go +++ b/internal/dataprovider/eventrule.go @@ -59,7 +59,7 @@ var ( ActionTypeDataRetentionCheck, ActionTypePasswordExpirationCheck, ActionTypeUserExpirationCheck, ActionTypeUserInactivityCheck, ActionTypeIDPAccountCheck, ActionTypeRotateLogs} // EnabledActionCommands defines the system commands that can be executed via EventManager, - // an empty list means that any command is allowed to be executed. + // an empty list means that no command is allowed to be executed. EnabledActionCommands []string ) @@ -455,9 +455,6 @@ func (c *EventActionHTTPConfig) GetHTTPClient() *http.Client { // IsActionCommandAllowed returns true if the specified command is allowed func IsActionCommandAllowed(cmd string) bool { - if len(EnabledActionCommands) == 0 { - return true - } return slices.Contains(EnabledActionCommands, cmd) } diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index b504d47a..9e1dd4c0 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -1840,6 +1840,10 @@ func TestBasicActionRulesHandling(t *testing.T) { }, }, } + dataprovider.EnabledActionCommands = []string{a.Options.CmdConfig.Cmd} + defer func() { + dataprovider.EnabledActionCommands = nil + }() _, _, err = httpdtest.UpdateEventAction(a, http.StatusOK) assert.NoError(t, err) // invalid type @@ -2374,13 +2378,24 @@ func TestEventActionValidation(t *testing.T) { assert.NoError(t, err) assert.Contains(t, string(resp), "command is required") action.Options.CmdConfig.Cmd = "relative" + dataprovider.EnabledActionCommands = []string{action.Options.CmdConfig.Cmd} + defer func() { + dataprovider.EnabledActionCommands = nil + }() + _, resp, err = httpdtest.AddEventAction(action, http.StatusBadRequest) assert.NoError(t, err) assert.Contains(t, string(resp), "invalid command, it must be an absolute path") action.Options.CmdConfig.Cmd = filepath.Join(os.TempDir(), "cmd") _, resp, err = httpdtest.AddEventAction(action, http.StatusBadRequest) assert.NoError(t, err) + assert.Contains(t, string(resp), "is not allowed") + + dataprovider.EnabledActionCommands = []string{action.Options.CmdConfig.Cmd} + _, resp, err = httpdtest.AddEventAction(action, http.StatusBadRequest) + assert.NoError(t, err) assert.Contains(t, string(resp), "invalid command action timeout") + action.Options.CmdConfig.Timeout = 30 action.Options.CmdConfig.EnvVars = []dataprovider.KeyValue{ { @@ -24027,6 +24042,10 @@ func TestWebEventAction(t *testing.T) { }, }, } + dataprovider.EnabledActionCommands = []string{action.Options.CmdConfig.Cmd} + defer func() { + dataprovider.EnabledActionCommands = nil + }() form.Set("type", fmt.Sprintf("%d", action.Type)) req, err = http.NewRequest(http.MethodPost, path.Join(webAdminEventActionPath, action.Name), bytes.NewBuffer([]byte(form.Encode()))) diff --git a/templates/webadmin/eventaction.html b/templates/webadmin/eventaction.html index 84e5b9c5..75ffb795 100644 --- a/templates/webadmin/eventaction.html +++ b/templates/webadmin/eventaction.html @@ -44,6 +44,11 @@ explicit grant from the SFTPGo Team (support@sftpgo.com).
@@ -400,21 +405,13 @@ explicit grant from the SFTPGo Team (support@sftpgo.com).
- {{- range .EnabledCommands}} {{- end}}
- {{- else}} -
- -
- -
-
-
{{- end}}