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

Local run data dir #997

Merged
merged 4 commits into from
Mar 25, 2021
Merged

Local run data dir #997

merged 4 commits into from
Mar 25, 2021

Conversation

mssalvatore
Copy link
Collaborator

What does this PR do?

Uses the runtime-configurable data_dir to store the monkey agent executable when the monkey is run from the island.

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 documentation framework updated to reflect the changes?

Testing Checklist

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

    Tested by building an appimage and running

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

EnvironmentConfig needs to handle environment variables and '~' in its
`data_dir` property. Other components that consume `data_dir` need
environment variables and '~' resolved to an absolute path. Add a
property called `data_dir_abs_path` that calculates the absolute path
from `data_dir`. Since `data_dir` remains unchanged, the
EnvironmentConfig can be saved to file without modifying the `data_dir`
option in the file.
@codecov
Copy link

codecov bot commented Mar 24, 2021

Codecov Report

Merging #997 (b7df168) into appimage (52f80e8) will increase coverage by 0.31%.
The diff coverage is 52.74%.

Impacted file tree graph

@@             Coverage Diff              @@
##           appimage     #997      +/-   ##
============================================
+ Coverage     19.10%   19.42%   +0.31%     
============================================
  Files           337      338       +1     
  Lines         11487    11519      +32     
============================================
+ Hits           2195     2237      +42     
+ Misses         9292     9282      -10     
Impacted Files Coverage Δ
monkey/monkey_island.py 0.00% <0.00%> (ø)
monkey/monkey_island/cc/arg_parser.py 0.00% <0.00%> (ø)
...key_island/cc/environment/environment_singleton.py 71.87% <ø> (ø)
monkey/monkey_island/cc/main.py 0.00% <0.00%> (ø)
monkey/monkey_island/cc/resources/local_run.py 0.00% <0.00%> (ø)
...attack/technique_reports/technique_report_tools.py 0.00% <0.00%> (ø)
...island/cc/services/telemetry/processing/exploit.py 0.00% <0.00%> (ø)
...nd/cc/services/telemetry/processing/system_info.py 0.00% <0.00%> (ø)
monkey/monkey_island/cc/services/config.py 27.41% <10.00%> (ø)
monkey/monkey_island/cc/consts.py 100.00% <100.00%> (ø)
... and 7 more

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 2787c4c...b7df168. Read the comment docs.

@ghost
Copy link

ghost commented Mar 24, 2021

DeepCode failed to analyze this pull request

Something went wrong despite trying multiple times, sorry about that.
Please comment this pull request with "Retry DeepCode" to manually retry, or contact us so that a human can look into the issue.

Comment on lines +128 to +133
def set_home_env(monkeypatch, tmpdir):
monkeypatch.setenv("HOME", str(tmpdir))


def test_data_dir_abs_path_from_file(monkeypatch, tmpdir):
set_home_env(monkeypatch, tmpdir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not make set_home_env a fixture?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this case, both set_home_env() and the test need to use the same tmpdir. Passing a tmpdir to a function that sets an environment variable, to me, seems easier to understand than a fixture that relies on another fixture and returns a directory. Since it's only used in one test that's 3 lines long, it probably doesn't matter much one way or the other, but it's something to revisit if this test suite is expanded.

@mssalvatore mssalvatore merged commit a5160ee into appimage Mar 25, 2021
@mssalvatore mssalvatore deleted the local-run-data-dir branch April 15, 2021 11:31
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.

2 participants