Modify logic for user PBA command/file to allow both simultaneously #1020
Conversation
Codecov Report
@@ Coverage Diff @@
## release/1.10.0 #1020 +/- ##
==================================================
+ Coverage 27.64% 28.27% +0.63%
==================================================
Files 402 402
Lines 12833 12844 +11
==================================================
+ Hits 3548 3632 +84
+ Misses 9285 9212 -73
Continue to review full report at Codecov.
|
DeepCode failed to analyze this pull requestSomething went wrong despite trying multiple times, sorry about that. |
| if self.filename not in cmd: # PBA command is not about uploaded PBA file | ||
| file_path = os.path.join(get_monkey_dir_path(), WormConfiguration.PBA_linux_filename) | ||
| self.command_list.append(DEFAULT_LINUX_COMMAND.format(file_path)) |
There was a problem hiding this comment.
So now if the filename is not a part of the cmd, we run the file regardless?
There was a problem hiding this comment.
The fact that files didn't run if you wrote a command without running them was intentional. The improvement you made is still a welcome one, but we need to address the cause of this defect, which is the description of configuration fields. See if you can re-phrase them to be more concise AND more specific. I'll help.
|
@VakarisZ Do we need to rephrase after these changes? I think now the description makes sense.
|
|
My gut says this is a confusing interface to begin with, I don't think this change helps that. The file upload component can cause 2 things to happen:
I think that 1 component with 1 action will prevent the user from getting confused. Some malware are easy to identify because they leave specific files on the system. Suppose I have an antimalware solution that searches the machine for the existence of such a file. To simulate this kind of malware, I add a file to be uploaded. I don't want the file to be executed. I just want it to be copied to the victim machine so that I can test how long it takes my antimalware solution to respond. How do I achieve this with the current behavior or the behavior proposed in this PR? Additionally, there are a number of edge cases that we're likely not going do account for, which will cause the monkey to do something the user didn't quite expect. Since the monkey needs to be safe for production environments, it needs to be intuitive and predictable. I think we should never run the file provided. Instead, the user can provide a command that runs the file if they want the file to be executed. If we want to make it simple for a user to just upload and execute a file, we can add a checkbox with a label like "Run this file". Regardless, I think this is something to be discussed further and solved after the release. |
|
For now, I think the best solution would be to remove the "Automatic file running" altogether. This would cover the "upload only" case and make the workflow more streamlined: you must call the uploaded file yourself. |
|
Resolved in #1027 |
If the custom PBA command and custom PBA file fields are both set in the config, the previous logic wouldn't execute the custom PBA file. It would only be executed if the custom PBA command ran it.
Now, it checks whether the command has any reference of the file and executes the command + file or only the command accordingly.
Adding relevant unit tests is left.