-
Notifications
You must be signed in to change notification settings - Fork 38
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
Feature request: mark and log infected files. #295
base: master
Are you sure you want to change the base?
Conversation
…fected' files. This patch can easily used to move 'infected' files into a quarantain folder. Translations are incomplete. Signed-off-by: tx0h <[email protected]>
Hello there, We hope that the reviewing process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR reviewing process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! |
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.
Thank you for the PR.
This needs to be tested.
You can remove the translations from this PR. They are handled via Transifex and synced automatically to this repo.
} else { | ||
$msg = 'Infected file found.'; | ||
} | ||
$this->logError('mark and log'); |
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.
$this->logError('mark and log'); |
Too verbose.
* * Mark infected file | ||
*/ | ||
private function markFile(): void { | ||
$this->file->move($this->file->getPath() . " [\u{2623} VIRUS DETECTED \u{2620}]"); |
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.
$this->file->move($this->file->getPath() . " [\u{2623} VIRUS DETECTED \u{2620}]"); | |
$this->file->move($this->file->getPath() . " [VIRUS DETECTED]"); |
IMO, the unicode chars are a bit too much.
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.
What if the file is already marked? Will it get the same suffix again on the second scan?
E.g. some_file [VIRUS DETECTED] [VIRUS DETECTED]
@@ -116,6 +117,16 @@ public function processInfected(Status $status): void { | |||
} | |||
$this->logError($msg . ' ' . $status->getDetails()); | |||
$this->deleteFile(); | |||
} else if ($shouldMarkAndLog) { |
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.
} else if ($shouldMarkAndLog) { | |
} elseif ($shouldMarkAndLog) { |
Any update? |
Hello,
there are two options how to handle 'infected' with files_antivirus by now.
option 1: delete the 'infected' file
option 2: write about the 'infected' file in a log file
option 1 means, you risk to delete false flagged files and option 2 means, you need to take attention to an endless stream of log entries while the 'infected' file still resides in the cloud.
here is the 3rd option + the pull request:
Added infectedAction option 'Mark and log', which marks (renames) 'infected' files. This patch can easily modified to move 'infected' files into a quarantine folder. Translations are incomplete.
this option adds a string to the 'infected' files file name to mark it as infected. the users are now obviously warned about the suspected nature of the file.
best.