Skip to content

Commit 4799665

Browse files
asottileasottile
authored
Merge pull request #162 from Dunedan/improve-aws-credential-searching
Improve searching for configured AWS credentials
2 parents 9c0460b + 3939aee commit 4799665

9 files changed

Lines changed: 208 additions & 44 deletions

‎README.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,12 @@ Add this to your `.pre-commit-config.yaml`
4040
- `check-xml` - Attempts to load all xml files to verify syntax.
4141
- `check-yaml` - Attempts to load all yaml files to verify syntax.
4242
- `debug-statements` - Check for pdb / ipdb / pudb statements in code.
43-
- `detect-aws-credentials` - Checks for the existence of AWS secrets that you have set up with the AWS CLI.
43+
- `detect-aws-credentials` - Checks for the existence of AWS secrets that you
44+
have set up with the AWS CLI.
45+
The following arguments are available:
46+
- `--credentials-file` - additional AWS CLI style configuration file in a
47+
non-standard location to fetch configured credentials from. Can be repeated
48+
multiple times.
4449
- `detect-private-key` - Checks for the existence of private keys.
4550
- `double-quote-string-fixer` - This hook replaces double quoted strings
4651
with single quoted strings.

‎pre_commit_hooks/detect_aws_credentials.py‎

Lines changed: 79 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,62 +7,120 @@
77
from six.moves import configparser
88

99

10-
def get_your_keys(credentials_file):
11-
"""reads the secret keys in your credentials file in order to be able to
12-
look for them in the submitted code.
10+
def get_aws_credential_files_from_env():
11+
"""Extract credential file paths from environment variables."""
12+
files = set()
13+
for env_var in (
14+
'AWS_CONFIG_FILE', 'AWS_CREDENTIAL_FILE', 'AWS_SHARED_CREDENTIALS_FILE',
15+
'BOTO_CONFIG'
16+
):
17+
if env_var in os.environ:
18+
files.add(os.environ[env_var])
19+
return files
20+
21+
22+
def get_aws_secrets_from_env():
23+
"""Extract AWS secrets from environment variables."""
24+
keys = set()
25+
for env_var in (
26+
'AWS_SECRET_ACCESS_KEY', 'AWS_SECURITY_TOKEN', 'AWS_SESSION_TOKEN'
27+
):
28+
if env_var in os.environ:
29+
keys.add(os.environ[env_var])
30+
return keys
31+
32+
33+
def get_aws_secrets_from_file(credentials_file):
34+
"""Extract AWS secrets from configuration files.
35+
36+
Read an ini-style configuration file and return a set with all found AWS
37+
secret access keys.
1338
"""
1439
aws_credentials_file_path = os.path.expanduser(credentials_file)
1540
if not os.path.exists(aws_credentials_file_path):
16-
return None
41+
return set()
1742

1843
parser = configparser.ConfigParser()
19-
parser.read(aws_credentials_file_path)
44+
try:
45+
parser.read(aws_credentials_file_path)
46+
except configparser.MissingSectionHeaderError:
47+
return set()
2048

2149
keys = set()
2250
for section in parser.sections():
23-
keys.add(parser.get(section, 'aws_secret_access_key'))
51+
for var in (
52+
'aws_secret_access_key', 'aws_security_token',
53+
'aws_session_token'
54+
):
55+
try:
56+
keys.add(parser.get(section, var))
57+
except configparser.NoOptionError:
58+
pass
2459
return keys
2560

2661

2762
def check_file_for_aws_keys(filenames, keys):
63+
"""Check if files contain AWS secrets.
64+
65+
Return a list of all files containing AWS secrets and keys found, with all
66+
but the first four characters obfuscated to ease debugging.
67+
"""
2868
bad_files = []
2969

3070
for filename in filenames:
3171
with open(filename, 'r') as content:
3272
text_body = content.read()
33-
if any(key in text_body for key in keys):
34-
# naively match the entire file, low chance of incorrect collision
35-
bad_files.append(filename)
36-
73+
for key in keys:
74+
# naively match the entire file, low chance of incorrect
75+
# collision
76+
if key in text_body:
77+
bad_files.append({'filename': filename,
78+
'key': key[:4] + '*' * 28})
3779
return bad_files
3880

3981

4082
def main(argv=None):
4183
parser = argparse.ArgumentParser()
42-
parser.add_argument('filenames', nargs='*', help='Filenames to run')
84+
parser.add_argument('filenames', nargs='+', help='Filenames to run')
4385
parser.add_argument(
4486
'--credentials-file',
45-
default='~/.aws/credentials',
87+
dest='credential_files',
88+
action='append',
89+
default=['~/.aws/config', '~/.aws/credentials', '/etc/boto.cfg',
90+
'~/.boto'],
4691
help=(
47-
'location of aws credentials file from which to get the secret '
48-
"keys we're looking for"
49-
),
92+
'Location of additional AWS credential files from which to get '
93+
'secret keys from'
94+
)
5095
)
5196
args = parser.parse_args(argv)
52-
keys = get_your_keys(args.credentials_file)
97+
98+
credential_files = set(args.credential_files)
99+
100+
# Add the credentials files configured via environment variables to the set
101+
# of files to to gather AWS secrets from.
102+
credential_files |= get_aws_credential_files_from_env()
103+
104+
keys = set()
105+
for credential_file in credential_files:
106+
keys |= get_aws_secrets_from_file(credential_file)
107+
108+
# Secrets might be part of environment variables, so add such secrets to
109+
# the set of keys.
110+
keys |= get_aws_secrets_from_env()
111+
53112
if not keys:
54113
print(
55-
'No aws keys were configured at {0}\n'
56-
'Configure them with --credentials-file'.format(
57-
args.credentials_file,
58-
),
114+
'No AWS keys were found in the configured credential files and '
115+
'environment variables.\nPlease ensure you have the correct '
116+
'setting for --credentials-file'
59117
)
60118
return 2
61119

62120
bad_filenames = check_file_for_aws_keys(args.filenames, keys)
63121
if bad_filenames:
64122
for bad_file in bad_filenames:
65-
print('AWS secret key found: {0}'.format(bad_file))
123+
print('AWS secret found in {filename}: {key}'.format(**bad_file))
66124
return 1
67125
else:
68126
return 0

testing/resources/sample_aws_credentials renamed to testing/resources/aws_config_with_multiple_sections.ini

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# this is an aws credentials configuration file. obviously not real credentials :P
1+
# file with AWS access key ids, AWS secret access keys and AWS session tokens in multiple sections
22
[default]
33
aws_access_key_id = AKIASLARTIBARTFAST11
44
aws_secret_access_key = 7xebzorgm5143ouge9gvepxb2z70bsb2rtrh099e
@@ -8,3 +8,5 @@ aws_secret_access_key = z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb
88
[staging]
99
aws_access_key_id = AKIAJIMMINYCRICKET0A
1010
aws_secret_access_key = ixswosj8gz3wuik405jl9k3vdajsnxfhnpui38ez
11+
[test]
12+
aws_session_token = foo

testing/resources/with_secrets.txt renamed to testing/resources/aws_config_with_secret.ini

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
#file with a secret key, you'll notice it is a section of sample_aws_credentials
2-
1+
# file with an AWS access key id and an AWS secret access key
32
[production]
43
aws_access_key_id = AKIAVOGONSVOGONS0042
54
aws_secret_access_key = z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb

‎testing/resources/aws_config_with_secret_and_session_token.ini‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# file with an AWS access key id, an AWS secret access key and an AWS session token
2+
[production]
3+
aws_access_key_id = AKIAVOGONSVOGONS0042
4+
aws_secret_access_key = z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb
5+
aws_session_token = foo

‎testing/resources/aws_config_with_session_token.ini‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# file with an AWS session token
2+
[production]
3+
aws_session_token = foo

‎testing/resources/aws_config_without_secrets.ini‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# file with an AWS access key id but no AWS secret access key
2+
[production]
3+
aws_access_key_id = AKIASLARTIBARTFAST11

‎testing/resources/with_no_secrets.txt‎

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎tests/detect_aws_credentials_test.py‎

Lines changed: 108 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,132 @@
11
import pytest
2+
from mock import patch
23

4+
from pre_commit_hooks.detect_aws_credentials import get_aws_credential_files_from_env
5+
from pre_commit_hooks.detect_aws_credentials import get_aws_secrets_from_env
6+
from pre_commit_hooks.detect_aws_credentials import get_aws_secrets_from_file
37
from pre_commit_hooks.detect_aws_credentials import main
48
from testing.util import get_resource_path
59

610

7-
# Input filename, expected return value
8-
TESTS = (
9-
('with_no_secrets.txt', 0),
10-
('with_secrets.txt', 1),
11-
('nonsense.txt', 0),
12-
('ok_json.json', 0),
11+
@pytest.mark.parametrize(
12+
('env_vars', 'values'),
13+
(
14+
({}, set()),
15+
({'AWS_DUMMY_KEY': '/foo'}, set()),
16+
({'AWS_CONFIG_FILE': '/foo'}, {'/foo'}),
17+
({'AWS_CREDENTIAL_FILE': '/foo'}, {'/foo'}),
18+
({'AWS_SHARED_CREDENTIALS_FILE': '/foo'}, {'/foo'}),
19+
({'BOTO_CONFIG': '/foo'}, {'/foo'}),
20+
({'AWS_DUMMY_KEY': '/foo', 'AWS_CONFIG_FILE': '/bar'}, {'/bar'}),
21+
(
22+
{
23+
'AWS_DUMMY_KEY': '/foo', 'AWS_CONFIG_FILE': '/bar',
24+
'AWS_CREDENTIAL_FILE': '/baz'
25+
},
26+
{'/bar', '/baz'}
27+
),
28+
(
29+
{
30+
'AWS_CONFIG_FILE': '/foo', 'AWS_CREDENTIAL_FILE': '/bar',
31+
'AWS_SHARED_CREDENTIALS_FILE': '/baz'
32+
},
33+
{'/foo', '/bar', '/baz'}
34+
),
35+
),
1336
)
37+
def test_get_aws_credentials_file_from_env(env_vars, values):
38+
"""Test that reading credential files names from environment variables works."""
39+
with patch.dict('os.environ', env_vars, clear=True):
40+
assert get_aws_credential_files_from_env() == values
1441

1542

16-
@pytest.mark.parametrize(('filename', 'expected_retval'), TESTS)
43+
@pytest.mark.parametrize(
44+
('env_vars', 'values'),
45+
(
46+
({}, set()),
47+
({'AWS_DUMMY_KEY': 'foo'}, set()),
48+
({'AWS_SECRET_ACCESS_KEY': 'foo'}, {'foo'}),
49+
({'AWS_SECURITY_TOKEN': 'foo'}, {'foo'}),
50+
({'AWS_SESSION_TOKEN': 'foo'}, {'foo'}),
51+
({'AWS_DUMMY_KEY': 'foo', 'AWS_SECRET_ACCESS_KEY': 'bar'}, {'bar'}),
52+
(
53+
{'AWS_SECRET_ACCESS_KEY': 'foo', 'AWS_SECURITY_TOKEN': 'bar'},
54+
{'foo', 'bar'}
55+
),
56+
),
57+
)
58+
def test_get_aws_secrets_from_env(env_vars, values):
59+
"""Test that reading secrets from environment variables works."""
60+
with patch.dict('os.environ', env_vars, clear=True):
61+
assert get_aws_secrets_from_env() == values
62+
63+
64+
@pytest.mark.parametrize(
65+
('filename', 'expected_keys'),
66+
(
67+
(
68+
'aws_config_with_secret.ini',
69+
{'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb'}
70+
),
71+
('aws_config_with_session_token.ini', {'foo'}),
72+
('aws_config_with_secret_and_session_token.ini',
73+
{'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb', 'foo'}),
74+
(
75+
'aws_config_with_multiple_sections.ini',
76+
{
77+
'7xebzorgm5143ouge9gvepxb2z70bsb2rtrh099e',
78+
'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb',
79+
'ixswosj8gz3wuik405jl9k3vdajsnxfhnpui38ez',
80+
'foo'
81+
}
82+
),
83+
('aws_config_without_secrets.ini', set()),
84+
('nonsense.txt', set()),
85+
('ok_json.json', set()),
86+
),
87+
)
88+
def test_get_aws_secrets_from_file(filename, expected_keys):
89+
"""Test that reading secrets from files works."""
90+
keys = get_aws_secrets_from_file(get_resource_path(filename))
91+
assert keys == expected_keys
92+
93+
94+
@pytest.mark.parametrize(
95+
('filename', 'expected_retval'),
96+
(
97+
('aws_config_with_secret.ini', 1),
98+
('aws_config_with_session_token.ini', 1),
99+
('aws_config_with_multiple_sections.ini', 1),
100+
('aws_config_without_secrets.ini', 0),
101+
('nonsense.txt', 0),
102+
('ok_json.json', 0),
103+
),
104+
)
17105
def test_detect_aws_credentials(filename, expected_retval):
106+
"""Test if getting configured AWS secrets from files to be checked in works."""
107+
18108
# with a valid credentials file
19109
ret = main((
20110
get_resource_path(filename),
21-
"--credentials-file=testing/resources/sample_aws_credentials",
111+
"--credentials-file=testing/resources/aws_config_with_multiple_sections.ini",
22112
))
23113
assert ret == expected_retval
24114

25115

26-
def test_non_existent_credentials(capsys):
27-
# with a non-existent credentials file
116+
@patch('pre_commit_hooks.detect_aws_credentials.get_aws_secrets_from_file')
117+
@patch('pre_commit_hooks.detect_aws_credentials.get_aws_secrets_from_env')
118+
def test_non_existent_credentials(mock_secrets_env, mock_secrets_file, capsys):
119+
"""Test behavior with no configured AWS secrets."""
120+
mock_secrets_env.return_value = set()
121+
mock_secrets_file.return_value = set()
28122
ret = main((
29-
get_resource_path('with_secrets.txt'),
123+
get_resource_path('aws_config_without_secrets.ini'),
30124
"--credentials-file=testing/resources/credentailsfilethatdoesntexist"
31125
))
32126
assert ret == 2
33127
out, _ = capsys.readouterr()
34128
assert out == (
35-
'No aws keys were configured at '
36-
'testing/resources/credentailsfilethatdoesntexist\n'
37-
'Configure them with --credentials-file\n'
129+
'No AWS keys were found in the configured credential files '
130+
'and environment variables.\nPlease ensure you have the '
131+
'correct setting for --credentials-file\n'
38132
)

0 commit comments

Comments
 (0)