Merge pull request #3 from pre-commit/master

get up to date with upstream

.travis.yml

@@ -5,13 +5,9 @@ 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.
 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:

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

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:
+  - `--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.
@@ -48,13 +53,17 @@ 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.
-- `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`;

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:

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())

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

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
@@ -16,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 = [
@@ -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='*',

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())

pre_commit_hooks/detect_aws_credentials.py

@@ -7,65 +7,126 @@ 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_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
 
+
 if __name__ == '__main__':
     exit(main())

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',
 ]
 
 

pre_commit_hooks/fix_encoding_pragma.py

@@ -138,5 +138,6 @@ def main(argv=None):
 
     return retv
 
+
 if __name__ == "__main__":
     exit(main())

pre_commit_hooks/forbid_new_submodules.py

@@ -0,0 +1,33 @@
+from __future__ import absolute_import
+from __future__ import print_function
+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()
+        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())

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',
@@ -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',

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

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

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

testing/resources/aws_config_with_session_token.ini

@@ -0,0 +1,3 @@
+# file with an AWS session token
+[production]
+aws_session_token = foo

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

testing/resources/with_no_secrets.txt

@@ -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

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'
     )

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

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

tox.ini

@@ -1,11 +1,16 @@
 [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
 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}