detect-secrets-hook mutates baseline and fails

[lirantal]

Problem statement

When detect-secrets-hook runs and mutates the secrets baseline for timestamp and update of the line numbers it also terminates with exit code 1 which means an error, and looks like this:

The baseline file was updated.
Probably to keep line numbers of secrets up-to-date.
Please `git add .secrets-baseline`, thank you.

This means that if I use it in a pre-commit hooks setup it fails my commit process and requires me to manually re-add the secrets baseline and run it again. It is very unintuitive and requires manual work for developers.

Proposal

Show the same error message about baseline being up to date and adding it to the repository but terminate with exit code 0 to make sure a pre-commit script doesn't fail. This is also a strong point IMO because the error isn't that secrets were detected, just that the baseline gets mutated which is not an exception but rather expected behavior of the tool.

Alternative

If terminating with an error code is required at least use a different error code so a scripting on top of it can differentiate between the different errors and act accordingly.

[domanchi]

Our development of this hook was under the assumption that the hook would be used together with a specific pre-commit framework. IIRC, this framework does not display output unless the error code is non-zero.

I agree that this isn't the best option for it, but this is the design reason for that choice. If reducing manual effort is the problem, maybe we can auto-add this? The issue is that I'm not sure whether auto-adding is expected behavior with the framework.

Personally, I use the following script in my workflow:

function main() {
    # Run the pre-commit in the tox environment, to mitigate OS issues
    tox -e pre-commit -- run

    # Add changed files (assuming pre-commit modified files)
    if [[ $? == 1 ]]; then
        git diff --staged --name-only --diff-filter=ARM | xargs git add
    fi
}

main

As always, we're open to suggestions, but please keep this compatibility in mind with your changes.

[lirantal]

Other frameworks like lint-stated has the same kind of behavior and will not show a message unless there's an error. My current workaround for this is to wrap the detect-secrets-hook with my own to parse the stderr output and then return a 0 exit code. However as I mentioned, I don't think this specific circumstance is of an error'ish nature. The hook updates the timestamp/line numbers of the file and then requests to re-stage it - that's IMHO not something you want to break the commit for.

How about my suggestion of at least changing the exit code number so that we can differentiate between errors (1) and say file mutations like in this topic and exit with say (2) ?

[KevinHock]

The alternate exit code sounds great to me 👍

As an aside, once #92 is added you’ll have the option of detect-secrets not modifying the baseline on line number and timestamp changes.

[lirantal]

Sure thing, see PR at #214