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 @@ from six.moves import configparser 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 @@ def get_aws_secrets_from_file(credentials_file): 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 @@ def check_file_for_aws_keys(filenames, keys): # 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 @@ def main(argv=None): 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 pre_commit_hooks.detect_aws_credentials import main 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 @@ def test_detect_aws_credentials(filename, expected_retval): 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' + )