-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a build_permission_change_command method to ILinuxAgentCommandBuilder #10
Conversation
@@ -47,6 +47,12 @@ def build_download_command(self, download_options: LinuxDownloadOptions): | |||
:param download_options: Options needed for the command to be built | |||
""" | |||
|
|||
@abc.abstractmethod | |||
def build_chmod_command(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Should this be
build_set_permissions_command()
? I knowchmod
is normally how that's done, but there are conceivably other ways of getting the job done. - Should this accept a parameter for what the permissions should be? If so, should it be a single parameter or should we build an
Options
model like we do for the other two commands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If command builder is a utility to build command/bash statements then we should be specific. If there's another way to change permissions we'll have to add it to the interface, because it will be a different bash statement.
@@ -20,6 +20,11 @@ class LinuxDownloadOptions(InfectionMonkeyBaseModel): | |||
download_url: str | |||
|
|||
|
|||
class LinuxPermissionChangeOptions(InfectionMonkeyBaseModel): | |||
file_path: PurePosixPath | |||
permissions: int = Field(ge=0, le=777) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
permissions: int = Field(ge=0, le=777) | |
permissions: int = Field(ge=0, le=0o777) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: I recommend "LinuxChangePermissionsOptions"
def build_permission_change_command(self, | ||
permission_change_options: LinuxPermissionChangeOptions): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: I recommend "build_change_permissions_command"
0cc8393
to
a02c7f1
Compare
@@ -20,6 +20,11 @@ class LinuxDownloadOptions(InfectionMonkeyBaseModel): | |||
download_url: str | |||
|
|||
|
|||
class LinuxPermissionChangeOptions(InfectionMonkeyBaseModel): | |||
file_path: PurePosixPath | |||
permissions: int = Field(ge=0, le=777, default=700) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
permissions: int = Field(ge=0, le=777, default=700) | |
permissions: int = Field(ge=0, le=0o777, default=0o700) |
a02c7f1
to
2e781f8
Compare
vulture_allowlist.py
Outdated
ILinuxAgentCommandBuilder.build_permission_change_command | ||
ILinuxAgentCommandBuilder.permission_change_options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove vulture from this repository
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably. The right way to go about it is to make it an option in the cookiecutter template and then use cruft to update this repo and not include it.
Feel free to rename, but it's annoying since it has to be done manually, because it's a separate repository, so I prefer not to spend time on it |
53f9bf9
to
a7c51d0
Compare
The name of this member is changed to be consistent with the other `Options` models.
a7c51d0
to
0a0a4e6
Compare
0a0a4e6
to
a417515
Compare
Issue guardicore/monkey#4187