diff --git a/README.md b/README.md index 603e08c..51c4444 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,12 @@ Add this to your `.pre-commit-config.yaml` - `check-xml` - Attempts to load all xml files to verify syntax. - `check-yaml` - Attempts to load all yaml files to verify syntax. - `debug-statements` - Check for pdb / ipdb / pudb statements in code. -- `detect-aws-credentials` - Checks for the existence of AWS secrets that you have set up with the AWS CLI. +- `detect-aws-credentials` - Checks for the existence of AWS secrets that you + have set up with the AWS CLI. + The following arguments are available: + - `--credential-file` - additional AWS CLI style configuration file in a + non-standard location to fetch configured credentials from. Can be repeated + multiple times. - `detect-private-key` - Checks for the existence of private keys. - `double-quote-string-fixer` - This hook replaces double quoted strings with single quoted strings. diff --git a/pre_commit_hooks/detect_aws_credentials.py b/pre_commit_hooks/detect_aws_credentials.py index 9dda217..420333d 100644 --- a/pre_commit_hooks/detect_aws_credentials.py +++ b/pre_commit_hooks/detect_aws_credentials.py @@ -7,62 +7,118 @@ import os from six.moves import configparser -def get_your_keys(credentials_file): - """reads the secret keys in your credentials file in order to be able to - look for them in the submitted code. +def get_aws_credential_files_from_env(): + """Extract credential file paths from environment variables.""" + files = set() + for env_var in {'AWS_CREDENTIAL_FILE', 'AWS_SHARED_CREDENTIALS_FILE', + 'BOTO_CONFIG'}: + try: + 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: + keys.add(os.environ[env_var]) + except KeyError: + pass + return keys + + +def get_aws_secrets_from_file(credentials_file): + """Extract AWS secrets from configuration files. + + Read an ini-style configuration file and return a set with all found AWS + secret access keys. """ aws_credentials_file_path = os.path.expanduser(credentials_file) if not os.path.exists(aws_credentials_file_path): - return None + return set() parser = configparser.ConfigParser() - parser.read(aws_credentials_file_path) + try: + parser.read(aws_credentials_file_path) + except configparser.MissingSectionHeaderError: + return set() keys = set() for section in parser.sections(): - keys.add(parser.get(section, 'aws_secret_access_key')) + for var in {'aws_secret_access_key', 'aws_security_token', + 'aws_session_token'}: + try: + keys.add(parser.get(section, var)) + except configparser.NoOptionError: + pass return keys def check_file_for_aws_keys(filenames, keys): + """Check if files contain AWS secrets. + + Return a list of all files containing AWS secrets and keys found, with all + but the first four characters obfuscated to ease debugging. + """ bad_files = [] for filename in filenames: with open(filename, 'r') as content: text_body = content.read() - if any(key in text_body for key in keys): - # naively match the entire file, low chance of incorrect collision - bad_files.append(filename) - + for key in keys: + # naively match the entire file, low chance of incorrect + # collision + if key in text_body: + bad_files.append({'filename': filename, + 'key': key[:4].ljust(32, str('*'))}) return bad_files def main(argv=None): parser = argparse.ArgumentParser() - parser.add_argument('filenames', nargs='*', help='Filenames to run') + parser.add_argument('filenames', nargs='+', help='Filenames to run') parser.add_argument( '--credentials-file', - default='~/.aws/credentials', + dest='credential_files', + action='append', + default=['~/.aws/config', '~/.aws/credentials', '/etc/boto.cfg', + '~/.boto'], help=( - 'location of aws credentials file from which to get the secret ' - "keys we're looking for" - ), + 'Location of additional AWS credential files from which to get ' + 'secret keys from' + ) ) args = parser.parse_args(argv) - keys = get_your_keys(args.credentials_file) + + credential_files = set(args.credential_files) + + # Add the credentials files configured via environment variables to the set + # of files to to gather AWS secrets from. + credential_files |= get_aws_credential_files_from_env() + + keys = set() + for credential_file in credential_files: + keys |= get_aws_secrets_from_file(credential_file) + + # Secrets might be part of environment variables, so add such secrets to + # the set of keys. + keys |= get_aws_secrets_from_env() + if not keys: - print( - 'No aws keys were configured at {0}\n' - 'Configure them with --credentials-file'.format( - args.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 key found: {0}'.format(bad_file)) + print('AWS secret found in {filename}: {key}'.format( + **bad_file)) return 1 else: return 0 diff --git a/testing/resources/sample_aws_credentials b/testing/resources/aws_config_with_multiple_sections.ini similarity index 73% rename from testing/resources/sample_aws_credentials rename to testing/resources/aws_config_with_multiple_sections.ini index a79b021..ca6a8a3 100644 --- a/testing/resources/sample_aws_credentials +++ b/testing/resources/aws_config_with_multiple_sections.ini @@ -1,4 +1,4 @@ -# this is an aws credentials configuration file. obviously not real credentials :P +# file with AWS access key ids, AWS secret access keys and AWS session tokens in multiple sections [default] aws_access_key_id = AKIASLARTIBARTFAST11 aws_secret_access_key = 7xebzorgm5143ouge9gvepxb2z70bsb2rtrh099e @@ -8,3 +8,5 @@ aws_secret_access_key = z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb [staging] aws_access_key_id = AKIAJIMMINYCRICKET0A aws_secret_access_key = ixswosj8gz3wuik405jl9k3vdajsnxfhnpui38ez +[test] +aws_session_token = foo \ No newline at end of file diff --git a/testing/resources/with_secrets.txt b/testing/resources/aws_config_with_secret.ini similarity index 59% rename from testing/resources/with_secrets.txt rename to testing/resources/aws_config_with_secret.ini index 0018225..bb55017 100644 --- a/testing/resources/with_secrets.txt +++ b/testing/resources/aws_config_with_secret.ini @@ -1,5 +1,4 @@ -#file with a secret key, you'll notice it is a section of sample_aws_credentials - +# file with an AWS access key id and an AWS secret access key [production] aws_access_key_id = AKIAVOGONSVOGONS0042 aws_secret_access_key = z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb diff --git a/testing/resources/aws_config_with_secret_and_session_token.ini b/testing/resources/aws_config_with_secret_and_session_token.ini new file mode 100644 index 0000000..4bd675d --- /dev/null +++ b/testing/resources/aws_config_with_secret_and_session_token.ini @@ -0,0 +1,5 @@ +# file with an AWS access key id, an AWS secret access key and an AWS session token +[production] +aws_access_key_id = AKIAVOGONSVOGONS0042 +aws_secret_access_key = z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb +aws_session_token = foo \ No newline at end of file diff --git a/testing/resources/aws_config_with_session_token.ini b/testing/resources/aws_config_with_session_token.ini new file mode 100644 index 0000000..e07f2ac --- /dev/null +++ b/testing/resources/aws_config_with_session_token.ini @@ -0,0 +1,3 @@ +# file with an AWS session token +[production] +aws_session_token = foo \ No newline at end of file diff --git a/testing/resources/aws_config_without_secrets.ini b/testing/resources/aws_config_without_secrets.ini new file mode 100644 index 0000000..26d1692 --- /dev/null +++ b/testing/resources/aws_config_without_secrets.ini @@ -0,0 +1,3 @@ +# file with an AWS access key id but no AWS secret access key +[production] +aws_access_key_id = AKIASLARTIBARTFAST11 diff --git a/testing/resources/with_no_secrets.txt b/testing/resources/with_no_secrets.txt deleted file mode 100644 index d9ab505..0000000 --- a/testing/resources/with_no_secrets.txt +++ /dev/null @@ -1,5 +0,0 @@ -# file with an access key but no secrets -# you'll notice it is a redacted section of sample_aws_credentials - -[production] -aws_access_key_id = AKIASLARTIBARTFAST11 diff --git a/tests/detect_aws_credentials_test.py b/tests/detect_aws_credentials_test.py index 66513fe..410a33f 100644 --- a/tests/detect_aws_credentials_test.py +++ b/tests/detect_aws_credentials_test.py @@ -1,13 +1,70 @@ import pytest +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 +from pre_commit_hooks.detect_aws_credentials import get_aws_secrets_from_file 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): + """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_DUMMY_KEY', 'foobar') + assert get_aws_credential_files_from_env() == {'/foo', '/bar', '/baz'} + + +def test_get_aws_secrets_from_env(monkeypatch): + """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'} + + +@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 = ( - ('with_no_secrets.txt', 0), - ('with_secrets.txt', 1), + ('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), ) @@ -15,24 +72,30 @@ TESTS = ( @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.""" + # with a valid credentials file ret = main(( get_resource_path(filename), - "--credentials-file=testing/resources/sample_aws_credentials", + "--credentials-file=testing/resources/aws_config_with_multiple_sections.ini", )) assert ret == expected_retval -def test_non_existent_credentials(capsys): - # with a non-existent credentials file +def test_non_existent_credentials(capsys, monkeypatch): + """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()) ret = main(( - get_resource_path('with_secrets.txt'), + 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 configured at ' - 'testing/resources/credentailsfilethatdoesntexist\n' - 'Configure them with --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')