Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuring custom severities not working with pre-push #364

Closed
carhartl opened this issue May 25, 2022 · 2 comments · Fixed by #459
Closed

Configuring custom severities not working with pre-push #364

carhartl opened this issue May 25, 2022 · 2 comments · Fixed by #459

Comments

@carhartl
Copy link

Describe the bug
I don't seem to be able to make use of custom severities when using talisman within a pre-push hook.

To Reproduce
Steps to reproduce the behavior:

cat <<EOF > .talismanrc
custom_severities:
  - detector: HexContent
    severity: low
threshold: medium
EOF
echo 5ba6ef628df0b7c21e8d6bf6235d32914effa0d0de62d8ea96678316a1d5d32b > foo.txt
git commit -m "Test"
echo "refs/heads/main HEAD refs/heads/main HEAD^1" | talisman --githook pre-push

(Mimicking a pre-push hook in the last command.)

Output:

Talisman Scan: 3 / 3 <---------------------------------------------------------------------------------> 100.00%

Talisman Report:
+---------+----------------------------------------------------+----------+
|  FILE   |                       ERRORS                       | SEVERITY |
+---------+----------------------------------------------------+----------+
| foo.txt | Expected file to not to contain                    | high     |
|         | hex encoded texts such as:                         |          |
|         | 5ba6ef628df0b7c21e8d6bf6235d32914effa0d0de62d8e... |          |
+---------+----------------------------------------------------+----------+


If you are absolutely sure that you want to ignore the above files from talisman detectors, consider pasting the following format in .talismanrc file in the project root

fileignoreconfig:
- filename: foo.txt
  checksum: 15acfde30d57c8dda5f7b41007a55c446e9d0c79119b77794e48148b7d1a2e2c
version: ""

Talisman done in 51.337541ms

Expected behavior
Talisman does not report hex encoded text with high severity, no detections should have been reported respecting the threshold.

Environment:

  • OS: macOS Monterey 12.4
  • talisman 1.26.0
@lizc126
Copy link

lizc126 commented Nov 27, 2023

Hi, have you found any solution? Thanks!

zph added a commit to zph/talisman that referenced this issue Jun 22, 2024
Original ability was added in: thoughtworks#281

When trying out the functionality, it does not respect custom_severities
as documented in the README.

This patch addresses it by including the porting of custom severities
from the the same location where other config keys are copied

I've confirmed that it works both in a new test and in manual regression
testing.

Fixes: thoughtworks#364
zph added a commit to zph/talisman that referenced this issue Jun 22, 2024
Original ability was added in: thoughtworks#281

When trying out the functionality, it does not respect custom_severities
as documented in the README.

This patch addresses it by including the porting of custom severities
from the the same location where other config keys are copied

I've confirmed that it works both in a new test and in manual regression
testing.

Fixes: thoughtworks#364
zph added a commit to zph/talisman that referenced this issue Jun 22, 2024
Original ability was added in: thoughtworks#281

When trying out the functionality, it does not respect custom_severities
as documented in the README.

This patch addresses it by including the porting of custom severities
from the the same location where other config keys are copied

I've confirmed that it works both in a new test and in manual regression
testing.

Fixes: thoughtworks#364
@zph
Copy link
Contributor

zph commented Jun 22, 2024

@carhartl I took your reproduction case and validated that with my patch submitted in #459 it outputs the expected result and an exitcode of 0. There's a bit of nuance in that it WILL report it as "low" but won't fail the build by returning an exitcode != 0, which is the behavior I would want and what seems to be supported by the tool after this patch.

Thank you for having succinct test case I could use (in addition to my own).

❯ cat .talismanrc
  custom_severities:
    - detector: HexContent
      severity: low
  threshold: medium
❯ echo "refs/heads/main HEAD refs/heads/main HEAD^1" | ../dist/talisman --githook pre-push
Talisman Scan: 3 / 3 <----------------------------------------------------------------------------------------------------------------------------------------------------------> 100.00%

Talisman Warnings:
+-----------------+----------------------------------------------------+----------+
|      FILE       |                      WARNINGS                      | SEVERITY |
+-----------------+----------------------------------------------------+----------+
| testing/foo.txt | Expected file to not contain                       | low      |
|                 | hex encoded texts such as:                         |          |
|                 | 5ba6ef628df0b7c21e8d6bf6235d32914effa0d0de62d8e... |          |
+-----------------+----------------------------------------------------+----------+

Please review the above file(s) to make sure that no sensitive content is being pushed


Talisman done in 33.960458ms

talisman/testing on  testing [$!?]
❯ echo $status
0

@lizc126 When #459 is reviewed and merged it will work.

deepthirera pushed a commit to deepthirera/talisman that referenced this issue Jul 19, 2024
…works#459)

Original ability was added in: thoughtworks#281

When trying out the functionality, it does not respect custom_severities
as documented in the README.

This patch addresses it by including the porting of custom severities
from the the same location where other config keys are copied

I've confirmed that it works both in a new test and in manual regression
testing.

Fixes: thoughtworks#364
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants