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

Hide security-sensitive strings in VCS command log messages #6890

Merged
merged 4 commits into from
Aug 21, 2019
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Address more review comments; implement __ne__().
cjerdonek committed Aug 19, 2019
commit 98e7ad73208c5e1a99c818d8fb69d0f097b2a463
18 changes: 12 additions & 6 deletions src/pip/_internal/utils/misc.py
Original file line number Diff line number Diff line change
@@ -1241,35 +1241,41 @@ def redact_password_from_url(url):
class HiddenText(object):
def __init__(
self,
secret, # type: str
redacted=None, # type: Optional[str]
secret, # type: str
redacted, # type: str
):
# type: (...) -> None
if redacted is None:
redacted = '****'

self.secret = secret
self.redacted = redacted

def __repr__(self):
# type: (...) -> str
return '<HiddenText {!r}>'.format(str(self))

def __str__(self):
# type: (...) -> str
return self.redacted

# This is useful for testing.
def __eq__(self, other):
# type: (Any) -> bool
if type(self) != type(other):
return False

# The string being used for redaction doesn't also have to match,
# just the raw, original string.
return (self.secret == other.secret)

# We need to provide an explicit __ne__ implementation for Python 2.
# TODO: remove this when we drop PY2 support.
def __ne__(self, other):
# type: (Any) -> bool
return not self == other


def hide_value(value):
# type: (str) -> HiddenText
return HiddenText(value)
return HiddenText(value, redacted='****')


def hide_url(url):
50 changes: 42 additions & 8 deletions tests/unit/test_utils.py
Original file line number Diff line number Diff line change
@@ -1365,20 +1365,54 @@ def test_redact_password_from_url(auth_url, expected_url):

class TestHiddenText:

def test_default_redacted(self):
hidden = HiddenText('my-secret')
assert repr(hidden) == "<HiddenText '****'>"
assert str(hidden) == '****'
assert hidden.redacted == '****'
assert hidden.secret == 'my-secret'

def test_custom_redacted(self):
def test_basic(self):
"""
Test str(), repr(), and attribute access.
"""
hidden = HiddenText('my-secret', redacted='######')
assert repr(hidden) == "<HiddenText '######'>"
assert str(hidden) == '######'
assert hidden.redacted == '######'
assert hidden.secret == 'my-secret'

def test_equality_with_str(self):
"""
Test equality (and inequality) with str objects.
"""
hidden = HiddenText('secret', redacted='****')

# Test that the object doesn't compare equal to either its original
# or redacted forms.
assert hidden != hidden.secret
assert hidden.secret != hidden

assert hidden != hidden.redacted
assert hidden.redacted != hidden

def test_equality_same_secret(self):
"""
Test equality with an object having the same secret.
"""
# Choose different redactions for the two objects.
hidden1 = HiddenText('secret', redacted='****')
hidden2 = HiddenText('secret', redacted='####')

assert hidden1 == hidden2
# Also test __ne__. This assertion fails in Python 2 without
# defining HiddenText.__ne__.
assert not hidden1 != hidden2

def test_equality_different_secret(self):
"""
Test equality with an object having a different secret.
"""
hidden1 = HiddenText('secret-1', redacted='****')
hidden2 = HiddenText('secret-2', redacted='****')

assert hidden1 != hidden2
# Also test __eq__.
assert not hidden1 == hidden2


def test_hide_value():
hidden = hide_value('my-secret')
22 changes: 11 additions & 11 deletions tests/unit/test_vcs.py
Original file line number Diff line number Diff line change
@@ -6,7 +6,7 @@
from pip._vendor.packaging.version import parse as parse_version

from pip._internal.exceptions import BadCommand
from pip._internal.utils.misc import HiddenText, hide_url, hide_value
from pip._internal.utils.misc import hide_url, hide_value
from pip._internal.vcs import make_vcs_requirement_url
from pip._internal.vcs.bazaar import Bazaar
from pip._internal.vcs.git import Git, looks_like_hash
@@ -357,7 +357,7 @@ def test_git__make_rev_args(username, password, expected):
(None, None, []),
('user', None, ['--username', 'user']),
('user', hide_value('pass'),
['--username', 'user', '--password', HiddenText('pass')]),
['--username', 'user', '--password', hide_value('pass')]),
])
def test_subversion__make_rev_args(username, password, expected):
"""
@@ -376,10 +376,10 @@ def test_subversion__get_url_rev_options():
)
hidden_url = hide_url(secret_url)
url, rev_options = Subversion().get_url_rev_options(hidden_url)
assert url == HiddenText('https://svn.example.com/MyProject')
assert url == hide_url('https://svn.example.com/MyProject')
assert rev_options.rev == 'v1.0'
assert rev_options.extra_args == (
['--username', 'user', '--password', HiddenText('pass')]
['--username', 'user', '--password', hide_value('pass')]
)


@@ -527,23 +527,23 @@ def test_obtain(self):
self.svn.obtain(self.dest, hide_url(self.url))
self.assert_call_args([
'svn', 'checkout', '-q', '--non-interactive', '--username',
'username', '--password', HiddenText('password'),
HiddenText('http://svn.example.com/'), '/tmp/test',
'username', '--password', hide_value('password'),
hide_url('http://svn.example.com/'), '/tmp/test',
])

def test_export(self):
self.svn.export(self.dest, hide_url(self.url))
self.assert_call_args([
'svn', 'export', '--non-interactive', '--username', 'username',
'--password', HiddenText('password'),
HiddenText('http://svn.example.com/'), '/tmp/test',
'--password', hide_value('password'),
hide_url('http://svn.example.com/'), '/tmp/test',
])

def test_fetch_new(self):
self.svn.fetch_new(self.dest, hide_url(self.url), self.rev_options)
self.assert_call_args([
'svn', 'checkout', '-q', '--non-interactive',
HiddenText('svn+http://username:password@svn.example.com/'),
hide_url('svn+http://username:password@svn.example.com/'),
'/tmp/test',
])

@@ -552,15 +552,15 @@ def test_fetch_new_revision(self):
self.svn.fetch_new(self.dest, hide_url(self.url), rev_options)
self.assert_call_args([
'svn', 'checkout', '-q', '--non-interactive', '-r', '123',
HiddenText('svn+http://username:password@svn.example.com/'),
hide_url('svn+http://username:password@svn.example.com/'),
'/tmp/test',
])

def test_switch(self):
self.svn.switch(self.dest, hide_url(self.url), self.rev_options)
self.assert_call_args([
'svn', 'switch', '--non-interactive',
HiddenText('svn+http://username:password@svn.example.com/'),
hide_url('svn+http://username:password@svn.example.com/'),
'/tmp/test',
])