GitGuardian / ggshield

Find and fix 400+ types of hardcoded secrets and 70+ types of infrastructure-as-code misconfigurations.
https://gitguardian.com
MIT License
1.65k stars 147 forks source link

Do not crash if a commit has no author #866

Closed agateau-gg closed 6 months ago

agateau-gg commented 6 months ago

Context

There are some strange commits around there. This one for example, made ggshield crash because its Author field is mostly empty.

What has been done

Make commit parser accept empty authors, added unit-test.

Testing

Needs #867 for CI to be green.

codecov-commenter commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.81%. Comparing base (a16f080) to head (3b09986). Report is 1 commits behind head on main.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #866 +/- ## ======================================= Coverage 91.81% 91.81% ======================================= Files 170 170 Lines 7066 7066 ======================================= Hits 6488 6488 Misses 578 578 ``` | [Flag](https://app.codecov.io/gh/GitGuardian/ggshield/pull/866/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GitGuardian) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/GitGuardian/ggshield/pull/866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GitGuardian) | `91.81% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GitGuardian#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

agateau-gg commented 6 months ago

Looks good, I just have a question: would it be possible for a commit to have no date, similarly to malformed commits with no author?

I hope not 😅 .

The way to produce a commit with no author is to actually set the git config value user.name to an empty string. The same thing can be done with user.email (which was already handled).

One can't do the same (AFAIK) for the date. It's possible to change the commit and author date with the $GIT_COMMITTER_DATE and $GIT_AUTHOR_DATE environment variables, so I just experimented messing with them: if they are empty git ignores them and uses the current date. If they contain invalid values it fails with a fatal: invalid date format message. We should be good 🤞🏻.