[Improvement] add alert script path check #15752
Conversation
| @@ -69,6 +69,11 @@ private AlertResult executeShellScript(String title, String content) { | |||
| alertResult.setMessage("shell script not support windows os"); | |||
| return alertResult; | |||
| } | |||
| if (!scriptPath.endsWith(".sh")) { | |||
| alertResult.setMessage("shell script is invalid, only support .sh file"); | |||
There was a problem hiding this comment.
Is it possible for users to put something like reboot in the script?
There was a problem hiding this comment.
Yes, but it seems we can't limit user custom script content. If add a blacklist, it is easy to miss some keys.
There was a problem hiding this comment.
In that case, we have to deprecate this plugin in the future. I think users tend to deploy a centralized alert server, which is different from worker and makes the alert server more vulnerable.
|
@caishunfeng @ruanwenjun @SbloodyS What about we deprecate alert script plugin? |
Not necessarily in next release, maybe sometime in the future. |
|
LGTM, user can add their own plugins if needed. |
Purpose of the pull request
Brief change log
dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSender.javaVerify this pull request