Repository: pre-commit/pre-commit-hooks

Issue:
UnicodeDecodeError in detect-aws-credentials is unreadable
```
Detect AWS Credentials...................................................Failed
hookid: detect-aws-credentials

Traceback (most recent call last):
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/bin/detect-aws-credentials", line 11, in <module>
    sys.exit(main())
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/site-packages/pre_commit_hooks/detect_aws_credentials.py", line 133, in main
    bad_filenames = check_file_for_aws_keys(args.filenames, keys)
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/site-packages/pre_commit_hooks/detect_aws_credentials.py", line 74, in check_file_for_aws_keys
    text_body = content.read()
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/codecs.py", line 322, in decode
    (result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xeb in position 103: invalid continuation byte
Traceback (most recent call last):
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/bin/detect-aws-credentials", line 11, in <module>
    sys.exit(main())
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/site-packages/pre_commit_hooks/detect_aws_credentials.py", line 133, in main
    bad_filenames = check_file_for_aws_keys(args.filenames, keys)
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/site-packages/pre_commit_hooks/detect_aws_credentials.py", line 74, in check_file_for_aws_keys
    text_body = content.read()
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/codecs.py", line 322, in decode
    (result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xc0 in position 2346: invalid start byte
Traceback (most recent call last):
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/bin/detect-aws-credentials", line 11, in <module>
    sys.exit(main())
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/site-packages/pre_commit_hooks/detect_aws_credentials.py", line 133, in main
    bad_filenames = check_file_for_aws_keys(args.filenames, keys)
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/site-packages/pre_commit_hooks/detect_aws_credentials.py", line 74, in check_file_for_aws_keys
    text_body = content.read()
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/codecs.py", line 322, in decode
    (result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe9 in position 187: invalid continuation byte
Traceback (most recent call last):
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/bin/detect-aws-credentials", line 11, in <module>
    sys.exit(main())
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/site-packages/pre_commit_hooks/detect_aws_credentials.py", line 133, in main
    bad_filenames = check_file_for_aws_keys(args.filenames, keys)
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/site-packages/pre_commit_hooks/detect_aws_credentials.py", line 74, in check_file_for_aws_keys
    text_body = content.read()
  File "/home/ryan/.cache/pre-commit/repo8jj7q4iz/py_env-python3.7/lib/python3.7/codecs.py", line 322, in decode
    (result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe9 in position 167: invalid continuation byte
```

It rejected the commit, so that's good. And I'm not sure if I can recommend any improvements to the handling of invalid unicode since I haven't looked at the code in this module.

But for sure this error message could be more useful. There are 773 files in this commit and it would sure be nice to know which one(s) have this problem.

Edit: Ideally if there's an exception it would continue to the next file so all the files that couldn't be processed can be listed to the user before the commit is rejected. I'm not looking forward to fixing one file, doing `git commit`, fixing another... 500 times. ;)

Discussion:

Patch:
diff --git a/pre_commit_hooks/detect_aws_credentials.py b/pre_commit_hooks/detect_aws_credentials.py
index a744b6f..1663cfd 100644
--- a/pre_commit_hooks/detect_aws_credentials.py
+++ b/pre_commit_hooks/detect_aws_credentials.py
@@ -69,7 +69,7 @@ def get_aws_secrets_from_file(credentials_file: str) -> Set[str]:
 
 def check_file_for_aws_keys(
         filenames: Sequence[str],
-        keys: Set[str],
+        keys: Set[bytes],
 ) -> List[BadFile]:
     """Check if files contain AWS secrets.
 
@@ -79,13 +79,14 @@ def check_file_for_aws_keys(
     bad_files = []
 
     for filename in filenames:
-        with open(filename, 'r') as content:
+        with open(filename, 'rb') as content:
             text_body = content.read()
             for key in keys:
                 # naively match the entire file, low chance of incorrect
                 # collision
                 if key in text_body:
-                    bad_files.append(BadFile(filename, key[:4].ljust(28, '*')))
+                    key_hidden = key.decode()[:4].ljust(28, '*')
+                    bad_files.append(BadFile(filename, key_hidden))
     return bad_files
 
 
@@ -137,7 +138,8 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
         )
         return 2
 
-    bad_filenames = check_file_for_aws_keys(args.filenames, keys)
+    keys_b = {key.encode() for key in keys}
+    bad_filenames = check_file_for_aws_keys(args.filenames, keys_b)
     if bad_filenames:
         for bad_file in bad_filenames:
             print(f'AWS secret found in {bad_file.filename}: {bad_file.key}')


Test patch:
diff --git a/tests/detect_aws_credentials_test.py b/tests/detect_aws_credentials_test.py
index 46e5b36..41b7b0a 100644
--- a/tests/detect_aws_credentials_test.py
+++ b/tests/detect_aws_credentials_test.py
@@ -117,6 +117,19 @@ def test_detect_aws_credentials(filename, expected_retval):
     assert ret == expected_retval
 
 
+def test_allows_arbitrarily_encoded_files(tmpdir):
+    src_ini = tmpdir.join('src.ini')
+    src_ini.write(
+        '[default]\n'
+        'aws_access_key_id=AKIASDFASDF\n'
+        'aws_secret_Access_key=9018asdf23908190238123\n',
+    )
+    arbitrary_encoding = tmpdir.join('f')
+    arbitrary_encoding.write_binary(b'\x12\x9a\xe2\xf2')
+    ret = main((str(arbitrary_encoding), '--credentials-file', str(src_ini)))
+    assert ret == 0
+
+
 @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):