From 70e405ede2dce7593ac1e51d0c0530a763585df8 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 30 Nov 2016 09:56:42 -0800 Subject: [PATCH 01/15] Add a new hook to forbid new submodules --- get-git-lfs.py | 1 + pre_commit_hooks/check_added_large_files.py | 3 -- pre_commit_hooks/check_merge_conflict.py | 4 +-- pre_commit_hooks/detect_aws_credentials.py | 1 + pre_commit_hooks/fix_encoding_pragma.py | 1 + pre_commit_hooks/forbid_new_submodules.py | 31 ++++++++++++++++ tests/forbid_new_submodules_test.py | 39 +++++++++++++++++++++ 7 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 pre_commit_hooks/forbid_new_submodules.py create mode 100644 tests/forbid_new_submodules_test.py diff --git a/get-git-lfs.py b/get-git-lfs.py index f71b5e2..4897a56 100755 --- a/get-git-lfs.py +++ b/get-git-lfs.py @@ -34,5 +34,6 @@ def main(): shutil.copyfileobj(src_file, dest_file) os.chmod(DEST_PATH, 0o755) + if __name__ == '__main__': exit(main()) diff --git a/pre_commit_hooks/check_added_large_files.py b/pre_commit_hooks/check_added_large_files.py index 03e88b8..b65c32a 100644 --- a/pre_commit_hooks/check_added_large_files.py +++ b/pre_commit_hooks/check_added_large_files.py @@ -6,7 +6,6 @@ from __future__ import unicode_literals import argparse import math import os -import sys from pre_commit_hooks.util import added_files from pre_commit_hooks.util import CalledProcessError @@ -49,8 +48,6 @@ def find_large_added_files(filenames, maxkb): def main(argv=None): - argv = argv if argv is not None else sys.argv[1:] - parser = argparse.ArgumentParser() parser.add_argument( 'filenames', nargs='*', diff --git a/pre_commit_hooks/check_merge_conflict.py b/pre_commit_hooks/check_merge_conflict.py index 4a98843..d986998 100644 --- a/pre_commit_hooks/check_merge_conflict.py +++ b/pre_commit_hooks/check_merge_conflict.py @@ -2,7 +2,6 @@ from __future__ import print_function import argparse import os.path -import sys CONFLICT_PATTERNS = [ b'<<<<<<< ', @@ -41,5 +40,6 @@ def detect_merge_conflict(argv=None): return retcode + if __name__ == '__main__': - sys.exit(detect_merge_conflict()) + exit(detect_merge_conflict()) diff --git a/pre_commit_hooks/detect_aws_credentials.py b/pre_commit_hooks/detect_aws_credentials.py index 4c51546..9dda217 100644 --- a/pre_commit_hooks/detect_aws_credentials.py +++ b/pre_commit_hooks/detect_aws_credentials.py @@ -67,5 +67,6 @@ def main(argv=None): else: return 0 + if __name__ == '__main__': exit(main()) diff --git a/pre_commit_hooks/fix_encoding_pragma.py b/pre_commit_hooks/fix_encoding_pragma.py index 5dcff93..3bf234e 100644 --- a/pre_commit_hooks/fix_encoding_pragma.py +++ b/pre_commit_hooks/fix_encoding_pragma.py @@ -138,5 +138,6 @@ def main(argv=None): return retv + if __name__ == "__main__": exit(main()) diff --git a/pre_commit_hooks/forbid_new_submodules.py b/pre_commit_hooks/forbid_new_submodules.py new file mode 100644 index 0000000..19f85d0 --- /dev/null +++ b/pre_commit_hooks/forbid_new_submodules.py @@ -0,0 +1,31 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + +from pre_commit_hooks.util import cmd_output + + +def main(argv=None): + # `argv` is ignored, pre-commit will send us a list of files that we + # don't care about + added_diff = cmd_output( + 'git', 'diff', '--staged', '--diff-filter=A', '--raw', + ) + retv = 0 + for line in added_diff.splitlines(): + metadata, filename = line.split('\t', 1) + new_mode = metadata.split(' ')[1] + if new_mode == '160000': + print('{}: new submodule introduced'.format(filename)) + retv = 1 + + if retv: + print('This commit introduces new submodules.') + print('Did you unintentionally `git add .`?') + print('To fix: git rm {thesubmodule} # no trailing slash') + print('Also check .gitmodules') + + return retv + + +if __name__ == '__main__': + exit(main()) diff --git a/tests/forbid_new_submodules_test.py b/tests/forbid_new_submodules_test.py new file mode 100644 index 0000000..1750e00 --- /dev/null +++ b/tests/forbid_new_submodules_test.py @@ -0,0 +1,39 @@ +from __future__ import absolute_import + +import pytest +from pre_commit.util import cmd_output + +from pre_commit_hooks.forbid_new_submodules import main + + +@pytest.yield_fixture +def git_dir_with_git_dir(tmpdir): + with tmpdir.as_cwd(): + cmd_output('git', 'init', '.') + cmd_output('git', 'commit', '-m', 'init', '--allow-empty') + cmd_output('git', 'init', 'foo') + with tmpdir.join('foo').as_cwd(): + cmd_output('git', 'commit', '-m', 'init', '--allow-empty') + yield + + +@pytest.mark.parametrize( + 'cmd', + ( + # Actually add the submodule + ('git', 'submodule', 'add', './foo'), + # Sneaky submodule add (that doesn't show up in .gitmodules) + ('git', 'add', 'foo'), + ), +) +def test_main_new_submodule(git_dir_with_git_dir, capsys, cmd): + cmd_output(*cmd) + assert main() == 1 + out, _ = capsys.readouterr() + assert out.startswith('foo: new submodule introduced\n') + + +def test_main_no_new_submodule(git_dir_with_git_dir): + open('test.py', 'a+').close() + cmd_output('git', 'add', 'test.py') + assert main() == 0 From e3e43781738833bf64b5a3ba03c0981bae478f89 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 30 Nov 2016 09:59:37 -0800 Subject: [PATCH 02/15] pip dropped pypy3, so will we --- .travis.yml | 1 - tox.ini | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 41f9e7d..7bc5f29 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,6 @@ env: # These should match the tox env list - TOXENV=py34 - TOXENV=py35 - TOXENV=pypy - - TOXENV=pypy3 install: pip install coveralls tox script: tox # Special snowflake. Our tests depend on making real commits. diff --git a/tox.ini b/tox.ini index f2ac125..b196c13 100644 --- a/tox.ini +++ b/tox.ini @@ -1,7 +1,7 @@ [tox] project = pre_commit_hooks # These should match the travis env list -envlist = py27,py34,py35,pypy,pypy3 +envlist = py27,py34,py35,pypy [testenv] deps = -rrequirements-dev.txt From 4b928ab06b87c81be25f8cd78bf702bf70b4e28f Mon Sep 17 00:00:00 2001 From: Chris Kuehl Date: Wed, 30 Nov 2016 10:10:29 -0800 Subject: [PATCH 03/15] Add forbid-new-submodules to hooks.yaml --- README.md | 1 + hooks.yaml | 6 ++++++ pre_commit_hooks/forbid_new_submodules.py | 2 ++ setup.py | 1 + 4 files changed, 10 insertions(+) diff --git a/README.md b/README.md index ee77b25..860d50b 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,7 @@ Add this to your `.pre-commit-config.yaml` - `fix-encoding-pragma` - Add `# -*- coding: utf-8 -*-` to the top of python files. - To remove the coding pragma pass `--remove` (useful in a python3-only codebase) - `flake8` - Run flake8 on your python files. +- `forbid-new-submodules` - Prevent addition of new git submodules. - `name-tests-test` - Assert that files in tests/ end in `_test.py`. - Use `args: ['--django']` to match `test*.py` instead. - `pyflakes` - Run pyflakes on your python files. diff --git a/hooks.yaml b/hooks.yaml index 11a414c..0604585 100644 --- a/hooks.yaml +++ b/hooks.yaml @@ -117,6 +117,12 @@ entry: flake8 language: python files: \.py$ +- id: forbid-new-submodules + name: Forbid new submodules + language: python + entry: forbid-new-submodules + description: Prevent addition of new git submodules + files: '' - id: name-tests-test name: Tests should end in _test.py description: This verifies that test files are named correctly diff --git a/pre_commit_hooks/forbid_new_submodules.py b/pre_commit_hooks/forbid_new_submodules.py index 19f85d0..c9464cf 100644 --- a/pre_commit_hooks/forbid_new_submodules.py +++ b/pre_commit_hooks/forbid_new_submodules.py @@ -1,4 +1,5 @@ from __future__ import absolute_import +from __future__ import print_function from __future__ import unicode_literals from pre_commit_hooks.util import cmd_output @@ -19,6 +20,7 @@ def main(argv=None): retv = 1 if retv: + print() print('This commit introduces new submodules.') print('Did you unintentionally `git add .`?') print('To fix: git rm {thesubmodule} # no trailing slash') diff --git a/setup.py b/setup.py index f92f821..774bc2e 100644 --- a/setup.py +++ b/setup.py @@ -52,6 +52,7 @@ setup( 'double-quote-string-fixer = pre_commit_hooks.string_fixer:main', 'end-of-file-fixer = pre_commit_hooks.end_of_file_fixer:end_of_file_fixer', 'fix-encoding-pragma = pre_commit_hooks.fix_encoding_pragma:main', + 'forbid-new-submodules = pre_commit_hooks.forbid_new_submodules:main', 'name-tests-test = pre_commit_hooks.tests_should_end_in_test:validate_files', 'pretty-format-json = pre_commit_hooks.pretty_format_json:pretty_format_json', 'requirements-txt-fixer = pre_commit_hooks.requirements_txt_fixer:fix_requirements_txt', From 77a7bba2f93cfe1205fc7167ef8379cbc6fcbb37 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 30 Nov 2016 10:40:03 -0800 Subject: [PATCH 04/15] v0.6.1 --- CHANGELOG | 9 +++++++++ setup.py | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 8978413..b9f2331 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,12 @@ +0.6.1 +===== +- trailing-whitespace-hook: restore original file on catastrophic failure +- trailing-whitespace-hook: support crlf +- check-yaml: Use safe_load +- check-json: allow custom key sort +- check-json: display filename for non-utf8 files +- New hook: forbid-new-submodules + 0.6.0 ===== - Merge conflict detection no longer crashes on binary files diff --git a/setup.py b/setup.py index 774bc2e..fb6abad 100644 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ setup( name='pre_commit_hooks', description='Some out-of-the-box hooks for pre-commit.', url='https://github.com/pre-commit/pre-commit-hooks', - version='0.6.0', + version='0.6.1', author='Anthony Sottile', author_email='asottile@umich.edu', From 0637a50cc229964aca435d322c1858fdbc7283ba Mon Sep 17 00:00:00 2001 From: alzeih Date: Fri, 2 Dec 2016 12:35:28 +1300 Subject: [PATCH 05/15] Fix test error "fatal: empty ident name (for <(null)>) not allowed" This occurs when there is no global setting for git config options user.name and user.email An example of the error shown below: E pre_commit.util.CalledProcessError: Command: ('/usr/bin/git', 'commit', '-m', 'init', '--allow-empty') E Return code: 128 E Expected return code: 0 E Output: (none) E Errors: E E *** Please tell me who you are. E E Run E E git config --global user.email "you@example.com" E git config --global user.name "Your Name" E E to set your account's default identity. E Omit --global to set the identity only in this repository. E E fatal: empty ident name (for <(null)>) not allowed --- .travis.yml | 3 --- tox.ini | 5 +++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7bc5f29..3bec92f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,10 +7,7 @@ env: # These should match the tox env list - TOXENV=pypy install: pip install coveralls tox script: tox -# Special snowflake. Our tests depend on making real commits. before_install: - - git config --global user.name "Travis CI" - - git config --global user.email "user@example.com" # Install git-lfs for a test - './get-git-lfs.py && export PATH="/tmp/git-lfs:$PATH"' after_success: diff --git a/tox.ini b/tox.ini index b196c13..bf2eba2 100644 --- a/tox.ini +++ b/tox.ini @@ -6,6 +6,11 @@ envlist = py27,py34,py35,pypy [testenv] deps = -rrequirements-dev.txt passenv = HOME HOMEPATH PROGRAMDATA +setenv = + GIT_AUTHOR_NAME = "test" + GIT_COMMITTER_NAME = "test" + GIT_AUTHOR_EMAIL = "test@example.com" + GIT_COMMITTER_EMAIL = "test@example.com" commands = coverage erase coverage run -m pytest {posargs:tests} From 75283ae18f9b6dd0db322163017b6465bcf74d6a Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Thu, 1 Dec 2016 16:16:40 -0800 Subject: [PATCH 06/15] Also remove git variables from appveyor --- appveyor.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 30eab46..4bde8cc 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -10,10 +10,6 @@ install: # Not a C# project build: false -before_test: - - git config --global user.name "AppVeyor CI" - - git config --global user.email "user@example.com" - test_script: tox cache: From 96fb7fa10f2f4c11ed33482a9ad7474251e5e97f Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 7 Dec 2016 10:44:07 -0800 Subject: [PATCH 07/15] Document pretty-format-json. Resolves #156 --- README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 860d50b..603e08c 100644 --- a/README.md +++ b/README.md @@ -52,10 +52,13 @@ Add this to your `.pre-commit-config.yaml` - `name-tests-test` - Assert that files in tests/ end in `_test.py`. - Use `args: ['--django']` to match `test*.py` instead. - `pyflakes` - Run pyflakes on your python files. -- `pretty-format-json` - Checks that all your JSON files are pretty +- `pretty-format-json` - Checks that all your JSON files are pretty. "Pretty" + here means that keys are sorted and indented. You can configure this with + the following commandline options: - `--autofix` - automatically format json files + - `--indent ...` - Control the indentation (either a number for a number of spaces or a string of whitespace). Defaults to 4 spaces. - `--no-sort-keys` - when autofixing, retain the original key ordering (instead of sorting the keys) - - `--indent ...` - Control the indentation (either a number for a number of spaces or a string of whitespace). + - `--top-keys comma,separated,keys` - Keys to keep at the top of mappings. - `requirements-txt-fixer` - Sorts entries in requirements.txt - `trailing-whitespace` - Trims trailing whitespace. - Markdown linebreak trailing spaces preserved for `.md` and`.markdown`; From c549cb25a17b78fd0b38ea6c64b269c65f29f462 Mon Sep 17 00:00:00 2001 From: Daniel Roschka Date: Mon, 26 Dec 2016 19:09:53 +0100 Subject: [PATCH 08/15] Detect Ed25519 keys as well Ed255519 keys generated by OpenSSH contain "BEGIN OPENSSH PRIVATE KEY" as identifier. This commit adds coverage for such keys as well. --- pre_commit_hooks/detect_private_key.py | 1 + tests/detect_private_key_test.py | 1 + 2 files changed, 2 insertions(+) diff --git a/pre_commit_hooks/detect_private_key.py b/pre_commit_hooks/detect_private_key.py index 1a4f323..0c2eca4 100644 --- a/pre_commit_hooks/detect_private_key.py +++ b/pre_commit_hooks/detect_private_key.py @@ -7,6 +7,7 @@ BLACKLIST = [ b'BEGIN RSA PRIVATE KEY', b'BEGIN DSA PRIVATE KEY', b'BEGIN EC PRIVATE KEY', + b'BEGIN OPENSSH PRIVATE KEY' ] diff --git a/tests/detect_private_key_test.py b/tests/detect_private_key_test.py index 4f2bb93..c6558ba 100644 --- a/tests/detect_private_key_test.py +++ b/tests/detect_private_key_test.py @@ -7,6 +7,7 @@ TESTS = ( (b'-----BEGIN RSA PRIVATE KEY-----', 1), (b'-----BEGIN DSA PRIVATE KEY-----', 1), (b'-----BEGIN EC PRIVATE KEY-----', 1), + (b'-----BEGIN OPENSSH PRIVATE KEY-----', 1), (b'ssh-rsa DATA', 0), (b'ssh-dsa DATA', 0), # Some arbitrary binary data From cdb3e2e4bff2630db9d764652b117f4ed6919a6a Mon Sep 17 00:00:00 2001 From: Chris Kuehl Date: Mon, 26 Dec 2016 14:51:26 -0800 Subject: [PATCH 09/15] Add trailing comma --- pre_commit_hooks/detect_private_key.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pre_commit_hooks/detect_private_key.py b/pre_commit_hooks/detect_private_key.py index 0c2eca4..d187364 100644 --- a/pre_commit_hooks/detect_private_key.py +++ b/pre_commit_hooks/detect_private_key.py @@ -7,7 +7,7 @@ BLACKLIST = [ b'BEGIN RSA PRIVATE KEY', b'BEGIN DSA PRIVATE KEY', b'BEGIN EC PRIVATE KEY', - b'BEGIN OPENSSH PRIVATE KEY' + b'BEGIN OPENSSH PRIVATE KEY', ] From b0d4cdb1eeeff04fabe2196c05089bbdefa26047 Mon Sep 17 00:00:00 2001 From: Daniel Roschka Date: Fri, 30 Dec 2016 08:41:24 +0100 Subject: [PATCH 10/15] Improve searching for configured AWS credentials The previous approach for finding AWS credentials was pretty naive and only covered contents of a single file (~/.aws/credentials by default). The AWS CLI documentation states various other ways to configure credentials which weren't covered: https://docs.aws.amazon.com/cli/latest/topic/config-vars.html#credentials Even that aren't all ways, a look into the code shows: https://github.com/boto/botocore/blob/develop/botocore/credentials.py This commit changes the behavior so the hook will behave in a way that if the AWS CLI is able to obtain credentials from local files, the hook will find them as well. The changes in detail are: - detect AWS session tokens and handle them like secret keys. - always search credentials in the default AWS CLI file locations ( ~/.aws/config, ~/.aws/credentials, /etc/boto.cfg and ~/.boto) - detect AWS credentials configured via environment variables in AWS_SECRET_ACCESS_KEY, AWS_SECURITY_TOKEN and AWS_SESSION_TOKEN - check additional configuration files configured via environment variables (AWS_CREDENTIAL_FILE, AWS_SHARED_CREDENTIALS_FILE and BOTO_CONFIG) - print out the first four characters of each secret found in files to be checked in, to make it easier to figure out, what the secrets were, which were going to be checked in - improve error handling for parsing ini-files - improve tests There is a major functional change introduced by this commit: Locations the AWS CLI gets credentials from are always searched and there is no way to disable them. --credentials-file is still there to specify one or more additional files to search credentials in. It's the purpose of this hook to find and check files for found credentials, so it should work in any case. As this commit also improves error handling for not-existing or malformed configuration files, it should be no big deal. Receiving credentials via the EC2 and ECS meta data services is not covered intentionally, to not further increase the amount of changes in this commit and as it's probably an edge case anyway to have this hook running in such an environment. --- README.md | 7 +- pre_commit_hooks/detect_aws_credentials.py | 102 ++++++++++++++---- ... => aws_config_with_multiple_sections.ini} | 4 +- ...secrets.txt => aws_config_with_secret.ini} | 3 +- ...s_config_with_secret_and_session_token.ini | 5 + .../aws_config_with_session_token.ini | 3 + .../resources/aws_config_without_secrets.ini | 3 + testing/resources/with_no_secrets.txt | 5 - tests/detect_aws_credentials_test.py | 85 +++++++++++++-- 9 files changed, 174 insertions(+), 43 deletions(-) rename testing/resources/{sample_aws_credentials => aws_config_with_multiple_sections.ini} (73%) rename testing/resources/{with_secrets.txt => aws_config_with_secret.ini} (59%) create mode 100644 testing/resources/aws_config_with_secret_and_session_token.ini create mode 100644 testing/resources/aws_config_with_session_token.ini create mode 100644 testing/resources/aws_config_without_secrets.ini delete mode 100644 testing/resources/with_no_secrets.txt 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') From 0fd09bf67aef299764b8bc9777779ced6a5bfc69 Mon Sep 17 00:00:00 2001 From: Daniel Roschka Date: Fri, 30 Dec 2016 10:39:38 +0100 Subject: [PATCH 11/15] Add AWS_CONFIG_FILE to the environment variables Turns out there is an additional environment variable AWS_CONFIG_FILE, which gets evaluated for finding configuration files as well. This commit adds support for it. --- pre_commit_hooks/detect_aws_credentials.py | 4 ++-- tests/detect_aws_credentials_test.py | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pre_commit_hooks/detect_aws_credentials.py b/pre_commit_hooks/detect_aws_credentials.py index 420333d..ed895d6 100644 --- a/pre_commit_hooks/detect_aws_credentials.py +++ b/pre_commit_hooks/detect_aws_credentials.py @@ -10,8 +10,8 @@ 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_CREDENTIAL_FILE', 'AWS_SHARED_CREDENTIALS_FILE', - 'BOTO_CONFIG'}: + for env_var in {'AWS_CONFIG_FILE', 'AWS_CREDENTIAL_FILE', + 'AWS_SHARED_CREDENTIALS_FILE', 'BOTO_CONFIG'}: try: files.add(os.environ[env_var]) except KeyError: diff --git a/tests/detect_aws_credentials_test.py b/tests/detect_aws_credentials_test.py index 410a33f..a366f01 100644 --- a/tests/detect_aws_credentials_test.py +++ b/tests/detect_aws_credentials_test.py @@ -19,8 +19,10 @@ def test_get_aws_credentials_file_from_env(monkeypatch): 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'} + assert get_aws_credential_files_from_env() == {'/foo', '/bar', '/baz', '/xxx'} def test_get_aws_secrets_from_env(monkeypatch): From 53697318eef9110cd867691325f24fe9bb842a59 Mon Sep 17 00:00:00 2001 From: Daniel Roschka Date: Fri, 30 Dec 2016 16:53:09 +0100 Subject: [PATCH 12/15] Fix a typo in the README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 51c4444..b68dd22 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ Add this to your `.pre-commit-config.yaml` - `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 + - `--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. From 3939aee4a377d01b68c9bacda9596295cc35b79e Mon Sep 17 00:00:00 2001 From: Daniel Roschka Date: Tue, 3 Jan 2017 19:05:49 +0100 Subject: [PATCH 13/15] Address issues mentioned in review --- pre_commit_hooks/detect_aws_credentials.py | 38 ++--- tests/detect_aws_credentials_test.py | 155 ++++++++++++--------- 2 files changed, 112 insertions(+), 81 deletions(-) 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' + ) From a7971b7d26c8c2d1308826d1bd00a5c467341c2a Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Tue, 3 Jan 2017 12:56:22 -0800 Subject: [PATCH 14/15] appveyor installed git-lfs, fix coverage --- pre_commit_hooks/check_added_large_files.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pre_commit_hooks/check_added_large_files.py b/pre_commit_hooks/check_added_large_files.py index b65c32a..5ef7f22 100644 --- a/pre_commit_hooks/check_added_large_files.py +++ b/pre_commit_hooks/check_added_large_files.py @@ -15,7 +15,7 @@ from pre_commit_hooks.util import cmd_output def lfs_files(): try: # pragma: no cover (no git-lfs) lines = cmd_output('git', 'lfs', 'status', '--porcelain').splitlines() - except CalledProcessError: + except CalledProcessError: # pragma: no cover (with git-lfs) lines = [] modes_and_fileparts = [ From 5da199bb8d60f764c0f77a20b0a1dc3a7640bcdd Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Tue, 3 Jan 2017 13:13:44 -0800 Subject: [PATCH 15/15] Formatting fixups --- pre_commit_hooks/detect_aws_credentials.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pre_commit_hooks/detect_aws_credentials.py b/pre_commit_hooks/detect_aws_credentials.py index a7847b9..b0826ca 100644 --- a/pre_commit_hooks/detect_aws_credentials.py +++ b/pre_commit_hooks/detect_aws_credentials.py @@ -74,8 +74,9 @@ def check_file_for_aws_keys(filenames, 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}) + bad_files.append({ + 'filename': filename, 'key': key[:4] + '*' * 28, + }) return bad_files @@ -86,8 +87,9 @@ def main(argv=None): '--credentials-file', dest='credential_files', action='append', - default=['~/.aws/config', '~/.aws/credentials', '/etc/boto.cfg', - '~/.boto'], + default=[ + '~/.aws/config', '~/.aws/credentials', '/etc/boto.cfg', '~/.boto', + ], help=( 'Location of additional AWS credential files from which to get ' 'secret keys from'