Hide creds when saving and use netrc file #73
Conversation
|
Shouldn't netrc be used by default? Are there any other apps in which you have to explicitly enable netrc support? |
|
Hmmm, seems curl does it explicitly: https://ec.haxx.se/usingcurl-netrc.html Perhaps we could allow optional path? Eg: Otherwise very well, please update authors/changelog.rst. |
Codecov Report
@@ Coverage Diff @@
## master #73 +/- ##
=========================================
- Coverage 77.44% 75.54% -1.9%
=========================================
Files 16 16
Lines 1667 1693 +26
Branches 300 289 -11
=========================================
- Hits 1291 1279 -12
- Misses 315 348 +33
- Partials 61 66 +5
Continue to review full report at Codecov.
|
|
I implemented the optional path it a bit differently than you proposed, because it seemed to make more sense for the implementation. The default value if the Should I do something about the test coverage? |
|
Perhaps a 3 tests, one asserting the output has no visible creds and one with |
drebs
force-pushed
the
use-netrc-file
branch
2 times, most recently
from
a2eb9a0 to
f9d1c5b
Compare
|
I added tests for creds handling in different scenarios (no creds, creds in url, creds in netrc, creds in url superseding creds in netrc), and one test for host masking. Do you think simple tests like these are enough or have you thought about more complex setups? |
|
Alright, thanks! |
There were some changes needed in pytest-benchmark so we could successfully use it for soledad benchmarks graphing: ionelmc/pytest-benchmark#73 ionelmc/pytest-benchmark#74 ionelmc/pytest-benchmark#75 The contributions were accepted but not released yet, so this commit uses the code from upstream git repository's master branch.
This is related to #72.
When using
--benchmark-storagewith elasticsearch feature pytest-benchmark prints the whole URL when saving the data after tests finished. If the URL contains credentials and if the results of the run are public, then the credentials are leaked. The first commit in this pull request addresses this issue by masking eventual credentials before printing the URL.Note that this does not protect other leaks such as when using tox together with pytest, as tox will print the whole pytest command line before running it and leak credentials anyway. Our current workaround for this is to use CI environment variables to set the content of a
pytest.inifile with the relevant options for pytest. Those options are not printed by tox because they are loaded by pytest itself.The second commit of this pull request implements the use of the
~/.netrcfile by adding the--benchmark-netrcoption. Further work can be done to add a--benchmark-netrc-file(using the same semantics ascurl) to point to a specific file other than the default one in user's home directory.