UnicodeDecodeError in detect-aws-credentials is unreadable

[rpdelaney]

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. ;)

[asottile]

agreed, this can be improved.

Looks like there's some prior art here or in pyupgrade if you'd like to apply it to this (and others here!)

[acdha]

This also breaks pre-commit run --all-files — I was trying to test updated config and got that stack trace when it hit an image file.

[asottile]

What was your configuration? by default this hook only runs on files which are identified as [text]

[acdha]

I had just added this block:

repos:
    - repo: https://github.com/pre-commit/pre-commit-hooks
      rev: v2.1.0
      hooks:
          - id: check-byte-order-marker
          - id: trailing-whitespace
          - id: check-merge-conflict
          - id: flake8
          - id: check-json
          - id: detect-aws-credentials

I wanted to confirm that my new rules weren't going to fail any existing files in the repo so I started pre-commit run --all-files while I was working on the commit message and got that UnicodeDecodeError.

[asottile]

Is this in an open source repository I can look at?

[acdha]

It is now: LibraryOfCongress/concordia#774

[acdha]

I should note that it works fine in normal usage since the text filter is there. It's only in the --all-files case that it blows up.

[asottile]

it seems that .plist is being identified as a text file:

$ ~/opt/venv/bin/identify-cli cloudformation/images/architecture-overview.graffle/data.plist
["file", "non-executable", "plist", "text"]

due to this: https://github.com/chriskuehl/identify/blob/1af88acc98ca80bf0c716241e35544d18337540f/identify/extensions.py#L100

hmm now to figure out if it's that plist is not text or whether that file has an incorrect extension 🤔

[asottile]

Looks like the extension can be binary or text :/

identify will probably need to grow some logic to handle that

[acdha]

Yech — when I ran it under pdb, it hit favicon.ico, too. Let me see if I can reproduce it.

[acdha]

In this case I'm not sure that file needs to be in this repo but obviously that's not a great general answer.

[asottile]

I'm working on a fix for identify -- it needs to dynamically check binary / text for .plist files since apple's standard is nutso

[acdha]

As a long-time Mac sysadmin, rest assured that I feel your pain on that. Their tools left a LOT to be desired for anyone outside of the Objective C world.