Skip to content
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

Ransomware skip encrypt readme #1333

Merged
merged 6 commits into from
Jul 19, 2021
Merged

Conversation

mssalvatore
Copy link
Collaborator

What does this PR do?

Fixes #1304

PR Checklist

  • Have you added an explanation of what your changes do and why you'd like to include them?
  • Is the TravisCI build passing?
  • Was the CHANGELOG.md updated to reflect the changes?
  • Was the documentation framework updated to reflect the changes?

Testing Checklist

  • Added relevant unit tests?
  • Have you successfully tested your changes locally? Elaborate:

    Tested by running unit tests on windows and linux
    Tested by building monkey agents on jenkins and running on both windows and linux

  • If applicable, add screenshots or log transcripts of the feature working

@codecov
Copy link

codecov bot commented Jul 16, 2021

Codecov Report

Merging #1333 (52412ab) into develop (b1fe850) will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1333      +/-   ##
===========================================
+ Coverage    40.25%   40.31%   +0.06%     
===========================================
  Files          474      475       +1     
  Lines        13860    13874      +14     
===========================================
+ Hits          5579     5593      +14     
  Misses        8281     8281              
Impacted Files Coverage Δ
monkey/tests/utils.py 62.50% <ø> (-18.75%) ⬇️
monkey/common/utils/file_utils.py 100.00% <100.00%> (ø)
monkey/infection_monkey/ransomware/consts.py 100.00% <100.00%> (ø)
...nkey/infection_monkey/ransomware/file_selectors.py 100.00% <100.00%> (ø)
.../infection_monkey/ransomware/ransomware_payload.py 100.00% <100.00%> (ø)
monkey/tests/conftest.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1fe850...52412ab. Read the comment docs.

@mssalvatore mssalvatore requested review from shreyamalviya and VakarisZ and removed request for shreyamalviya July 19, 2021 01:10
if filepath.name != README_FILE_NAME:
return True

return get_file_sha256_hash(filepath) != README_SHA256_HASH
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it turns out that even on windows, the readme.txt dropped retains the unix line endings, good to know

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I even opened the file in notepad, added a space, and saved it and the line endings were not changed. This will likely vary from editor to editor.



@pytest.fixture(scope="session")
def stable_file(data_for_tests_dir) -> Path:
Copy link
Contributor

@VakarisZ VakarisZ Jul 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a typehint that data_for_tests_dir is a Path as well. If we pass something else, the syntax of some_path / "some_file.txt" will break

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data_for_tests_dir isn't actually a Path in this case, it's a pytest fixture. There are type hints for fixtures, but you have to import them from a file that's intended to be private. pytest-dev/pytest#5981 (comment)

@@ -53,3 +55,12 @@ def test_directories_not_selected(ransomware_test_data, file_selector):
selected_files = file_selector(ransomware_test_data)

assert (ransomware_test_data / SUBDIR / HELLO_TXT) not in selected_files


def test_ransomware_readme_not_selected(ransomware_target, file_selector):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we add a test where README.txt get's selected if contents are different?

Copy link
Contributor

@VakarisZ VakarisZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but maybe could be improved by adding a test that asserts that we encrypt/don't filter out the readme file if it's contents are not the same as our file's

@mssalvatore mssalvatore force-pushed the ransomware-skip-encrypt-readme branch from b45cf6c to 52412ab Compare July 19, 2021 10:51
@mssalvatore mssalvatore merged commit 81f7de7 into develop Jul 19, 2021
@mssalvatore mssalvatore deleted the ransomware-skip-encrypt-readme branch July 19, 2021 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Readme.txt.monkey blocks file encryption
3 participants