Merge pull request #162 from Dunedan/improve-aws-credential-searching
Improve searching for configured AWS credentials
diff --git a/README.md b/README.md
index 603e08c..b68dd22 100644
--- a/README.md
+++ b/README.md
@@ -40,7 +40,12 @@
- `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:
+ - `--credentials-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..a7847b9 100644
--- a/pre_commit_hooks/detect_aws_credentials.py
+++ b/pre_commit_hooks/detect_aws_credentials.py
@@ -7,62 +7,120 @@
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_CONFIG_FILE', 'AWS_CREDENTIAL_FILE', 'AWS_SHARED_CREDENTIALS_FILE',
+ 'BOTO_CONFIG'
+ ):
+ if env_var in os.environ:
+ files.add(os.environ[env_var])
+ 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'
+ ):
+ if env_var in os.environ:
+ keys.add(os.environ[env_var])
+ 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] + '*' * 28})
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,
- ),
+ '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 72%
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 @@
[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/aws_config_with_secret.ini b/testing/resources/aws_config_with_secret.ini
new file mode 100644
index 0000000..bb55017
--- /dev/null
+++ b/testing/resources/aws_config_with_secret.ini
@@ -0,0 +1,4 @@
+# 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/testing/resources/with_secrets.txt b/testing/resources/with_secrets.txt
deleted file mode 100644
index 0018225..0000000
--- a/testing/resources/with_secrets.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-#file with a secret key, you'll notice it is a section of sample_aws_credentials
-
-[production]
-aws_access_key_id = AKIAVOGONSVOGONS0042
-aws_secret_access_key = z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb
diff --git a/tests/detect_aws_credentials_test.py b/tests/detect_aws_credentials_test.py
index 66513fe..9c2fda7 100644
--- a/tests/detect_aws_credentials_test.py
+++ b/tests/detect_aws_credentials_test.py
@@ -1,38 +1,132 @@
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
+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
-# Input filename, expected return value
-TESTS = (
- ('with_no_secrets.txt', 0),
- ('with_secrets.txt', 1),
- ('nonsense.txt', 0),
- ('ok_json.json', 0),
+@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."""
+ with patch.dict('os.environ', env_vars, clear=True):
+ assert get_aws_credential_files_from_env() == values
-@pytest.mark.parametrize(('filename', 'expected_retval'), TESTS)
+@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."""
+ 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()),
+ ),
+)
+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
+
+
+@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),
+ ),
+)
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
+@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."""
+ mock_secrets_env.return_value = set()
+ mock_secrets_file.return_value = 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'
+ 'No AWS keys were found in the configured credential files '
+ 'and environment variables.\nPlease ensure you have the '
+ 'correct setting for --credentials-file\n'
)
