Address issues mentioned in review
diff --git a/pre_commit_hooks/detect_aws_credentials.py b/pre_commit_hooks/detect_aws_credentials.py
index ed895d6..a7847b9 100644
--- a/pre_commit_hooks/detect_aws_credentials.py
+++ b/pre_commit_hooks/detect_aws_credentials.py
@@ -10,24 +10,23 @@
def get_aws_credential_files_from_env():
"""Extract credential file paths from environment variables."""
files = set()
- for env_var in {'AWS_CONFIG_FILE', 'AWS_CREDENTIAL_FILE',
- 'AWS_SHARED_CREDENTIALS_FILE', 'BOTO_CONFIG'}:
- try:
+ for env_var in (
+ 'AWS_CONFIG_FILE', 'AWS_CREDENTIAL_FILE', 'AWS_SHARED_CREDENTIALS_FILE',
+ 'BOTO_CONFIG'
+ ):
+ if env_var in os.environ:
files.add(os.environ[env_var])
- except KeyError:
- pass
return files
def get_aws_secrets_from_env():
"""Extract AWS secrets from environment variables."""
keys = set()
- for env_var in {'AWS_SECRET_ACCESS_KEY', 'AWS_SECURITY_TOKEN',
- 'AWS_SESSION_TOKEN'}:
- try:
+ for env_var in (
+ 'AWS_SECRET_ACCESS_KEY', 'AWS_SECURITY_TOKEN', 'AWS_SESSION_TOKEN'
+ ):
+ if env_var in os.environ:
keys.add(os.environ[env_var])
- except KeyError:
- pass
return keys
@@ -49,8 +48,10 @@
keys = set()
for section in parser.sections():
- for var in {'aws_secret_access_key', 'aws_security_token',
- 'aws_session_token'}:
+ for var in (
+ 'aws_secret_access_key', 'aws_security_token',
+ 'aws_session_token'
+ ):
try:
keys.add(parser.get(section, var))
except configparser.NoOptionError:
@@ -74,7 +75,7 @@
# collision
if key in text_body:
bad_files.append({'filename': filename,
- 'key': key[:4].ljust(32, str('*'))})
+ 'key': key[:4] + '*' * 28})
return bad_files
@@ -109,16 +110,17 @@
keys |= get_aws_secrets_from_env()
if not keys:
- print('No AWS keys were found in the configured credential files and '
- 'environment variables.\nPlease ensure you have the correct '
- 'setting for --credentials-file')
+ print(
+ 'No AWS keys were found in the configured credential files and '
+ 'environment variables.\nPlease ensure you have the correct '
+ 'setting for --credentials-file'
+ )
return 2
bad_filenames = check_file_for_aws_keys(args.filenames, keys)
if bad_filenames:
for bad_file in bad_filenames:
- print('AWS secret found in {filename}: {key}'.format(
- **bad_file))
+ print('AWS secret found in {filename}: {key}'.format(**bad_file))
return 1
else:
return 0
diff --git a/tests/detect_aws_credentials_test.py b/tests/detect_aws_credentials_test.py
index a366f01..9c2fda7 100644
--- a/tests/detect_aws_credentials_test.py
+++ b/tests/detect_aws_credentials_test.py
@@ -1,4 +1,5 @@
import pytest
+from mock import patch
from pre_commit_hooks.detect_aws_credentials import get_aws_credential_files_from_env
from pre_commit_hooks.detect_aws_credentials import get_aws_secrets_from_env
@@ -7,72 +8,100 @@
from testing.util import get_resource_path
-def test_get_aws_credentials_file_from_env(monkeypatch):
+@pytest.mark.parametrize(
+ ('env_vars', 'values'),
+ (
+ ({}, set()),
+ ({'AWS_DUMMY_KEY': '/foo'}, set()),
+ ({'AWS_CONFIG_FILE': '/foo'}, {'/foo'}),
+ ({'AWS_CREDENTIAL_FILE': '/foo'}, {'/foo'}),
+ ({'AWS_SHARED_CREDENTIALS_FILE': '/foo'}, {'/foo'}),
+ ({'BOTO_CONFIG': '/foo'}, {'/foo'}),
+ ({'AWS_DUMMY_KEY': '/foo', 'AWS_CONFIG_FILE': '/bar'}, {'/bar'}),
+ (
+ {
+ 'AWS_DUMMY_KEY': '/foo', 'AWS_CONFIG_FILE': '/bar',
+ 'AWS_CREDENTIAL_FILE': '/baz'
+ },
+ {'/bar', '/baz'}
+ ),
+ (
+ {
+ 'AWS_CONFIG_FILE': '/foo', 'AWS_CREDENTIAL_FILE': '/bar',
+ 'AWS_SHARED_CREDENTIALS_FILE': '/baz'
+ },
+ {'/foo', '/bar', '/baz'}
+ ),
+ ),
+)
+def test_get_aws_credentials_file_from_env(env_vars, values):
"""Test that reading credential files names from environment variables works."""
- monkeypatch.delenv('AWS_CREDENTIAL_FILE', raising=False)
- monkeypatch.delenv('AWS_SHARED_CREDENTIALS_FILE', raising=False)
- monkeypatch.delenv('BOTO_CONFIG', raising=False)
- assert get_aws_credential_files_from_env() == set()
- monkeypatch.setenv('AWS_CREDENTIAL_FILE', '/foo')
- assert get_aws_credential_files_from_env() == {'/foo'}
- monkeypatch.setenv('AWS_SHARED_CREDENTIALS_FILE', '/bar')
- assert get_aws_credential_files_from_env() == {'/foo', '/bar'}
- monkeypatch.setenv('BOTO_CONFIG', '/baz')
- assert get_aws_credential_files_from_env() == {'/foo', '/bar', '/baz'}
- monkeypatch.setenv('AWS_CONFIG_FILE', '/xxx')
- assert get_aws_credential_files_from_env() == {'/foo', '/bar', '/baz', '/xxx'}
- monkeypatch.setenv('AWS_DUMMY_KEY', 'foobar')
- assert get_aws_credential_files_from_env() == {'/foo', '/bar', '/baz', '/xxx'}
+ with patch.dict('os.environ', env_vars, clear=True):
+ assert get_aws_credential_files_from_env() == values
-def test_get_aws_secrets_from_env(monkeypatch):
+@pytest.mark.parametrize(
+ ('env_vars', 'values'),
+ (
+ ({}, set()),
+ ({'AWS_DUMMY_KEY': 'foo'}, set()),
+ ({'AWS_SECRET_ACCESS_KEY': 'foo'}, {'foo'}),
+ ({'AWS_SECURITY_TOKEN': 'foo'}, {'foo'}),
+ ({'AWS_SESSION_TOKEN': 'foo'}, {'foo'}),
+ ({'AWS_DUMMY_KEY': 'foo', 'AWS_SECRET_ACCESS_KEY': 'bar'}, {'bar'}),
+ (
+ {'AWS_SECRET_ACCESS_KEY': 'foo', 'AWS_SECURITY_TOKEN': 'bar'},
+ {'foo', 'bar'}
+ ),
+ ),
+)
+def test_get_aws_secrets_from_env(env_vars, values):
"""Test that reading secrets from environment variables works."""
- monkeypatch.delenv('AWS_SECRET_ACCESS_KEY', raising=False)
- monkeypatch.delenv('AWS_SESSION_TOKEN', raising=False)
- assert get_aws_secrets_from_env() == set()
- monkeypatch.setenv('AWS_SECRET_ACCESS_KEY', 'foo')
- assert get_aws_secrets_from_env() == {'foo'}
- monkeypatch.setenv('AWS_SESSION_TOKEN', 'bar')
- assert get_aws_secrets_from_env() == {'foo', 'bar'}
- monkeypatch.setenv('AWS_SECURITY_TOKEN', 'baz')
- assert get_aws_secrets_from_env() == {'foo', 'bar', 'baz'}
- monkeypatch.setenv('AWS_DUMMY_KEY', 'baz')
- assert get_aws_secrets_from_env() == {'foo', 'bar', 'baz'}
+ with patch.dict('os.environ', env_vars, clear=True):
+ assert get_aws_secrets_from_env() == values
-@pytest.mark.parametrize(('filename', 'expected_keys'), (
- ('aws_config_with_secret.ini', {
- 'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb'}),
- ('aws_config_with_session_token.ini', {'foo'}),
- ('aws_config_with_secret_and_session_token.ini',
- {'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb', 'foo'}),
- ('aws_config_with_multiple_sections.ini', {
- '7xebzorgm5143ouge9gvepxb2z70bsb2rtrh099e',
- 'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb',
- 'ixswosj8gz3wuik405jl9k3vdajsnxfhnpui38ez',
- 'foo'}),
- ('aws_config_without_secrets.ini', set()),
- ('nonsense.txt', set()),
- ('ok_json.json', set()),
-))
+@pytest.mark.parametrize(
+ ('filename', 'expected_keys'),
+ (
+ (
+ 'aws_config_with_secret.ini',
+ {'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb'}
+ ),
+ ('aws_config_with_session_token.ini', {'foo'}),
+ ('aws_config_with_secret_and_session_token.ini',
+ {'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb', 'foo'}),
+ (
+ 'aws_config_with_multiple_sections.ini',
+ {
+ '7xebzorgm5143ouge9gvepxb2z70bsb2rtrh099e',
+ 'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb',
+ 'ixswosj8gz3wuik405jl9k3vdajsnxfhnpui38ez',
+ 'foo'
+ }
+ ),
+ ('aws_config_without_secrets.ini', set()),
+ ('nonsense.txt', set()),
+ ('ok_json.json', set()),
+ ),
+)
def test_get_aws_secrets_from_file(filename, expected_keys):
"""Test that reading secrets from files works."""
keys = get_aws_secrets_from_file(get_resource_path(filename))
assert keys == expected_keys
-# Input filename, expected return value
-TESTS = (
- ('aws_config_with_secret.ini', 1),
- ('aws_config_with_session_token.ini', 1),
- ('aws_config_with_multiple_sections.ini', 1),
- ('aws_config_without_secrets.ini', 0),
- ('nonsense.txt', 0),
- ('ok_json.json', 0),
+@pytest.mark.parametrize(
+ ('filename', 'expected_retval'),
+ (
+ ('aws_config_with_secret.ini', 1),
+ ('aws_config_with_session_token.ini', 1),
+ ('aws_config_with_multiple_sections.ini', 1),
+ ('aws_config_without_secrets.ini', 0),
+ ('nonsense.txt', 0),
+ ('ok_json.json', 0),
+ ),
)
-
-
-@pytest.mark.parametrize(('filename', 'expected_retval'), TESTS)
def test_detect_aws_credentials(filename, expected_retval):
"""Test if getting configured AWS secrets from files to be checked in works."""
@@ -84,20 +113,20 @@
assert ret == expected_retval
-def test_non_existent_credentials(capsys, monkeypatch):
+@patch('pre_commit_hooks.detect_aws_credentials.get_aws_secrets_from_file')
+@patch('pre_commit_hooks.detect_aws_credentials.get_aws_secrets_from_env')
+def test_non_existent_credentials(mock_secrets_env, mock_secrets_file, capsys):
"""Test behavior with no configured AWS secrets."""
- monkeypatch.setattr(
- 'pre_commit_hooks.detect_aws_credentials.get_aws_secrets_from_env',
- lambda: set())
- monkeypatch.setattr(
- 'pre_commit_hooks.detect_aws_credentials.get_aws_secrets_from_file',
- lambda x: set())
+ mock_secrets_env.return_value = set()
+ mock_secrets_file.return_value = set()
ret = main((
get_resource_path('aws_config_without_secrets.ini'),
"--credentials-file=testing/resources/credentailsfilethatdoesntexist"
))
assert ret == 2
out, _ = capsys.readouterr()
- assert out == ('No AWS keys were found in the configured credential files '
- 'and environment variables.\nPlease ensure you have the '
- 'correct setting for --credentials-file\n')
+ assert out == (
+ 'No AWS keys were found in the configured credential files '
+ 'and environment variables.\nPlease ensure you have the '
+ 'correct setting for --credentials-file\n'
+ )
