GitGuardian / ggshield

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

chore: ensure no token is present before the test #796

Closed GG-HH closed 9 months ago

GG-HH commented 10 months ago

Please note that this is a bad fix because the isolated fs is not isolated and this should be fixed.

If I run this test alone with a hmsl_token in my file system, then it fails.

Please note that running the test will delete your current HMSL token if any.

codecov-commenter commented 10 months ago

Codecov Report

Merging #796 (4781871) into main (cbfbf3a) will not change coverage. The diff coverage is n/a.

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

@@           Coverage Diff           @@
##             main     #796   +/-   ##
=======================================
  Coverage   91.90%   91.90%           
=======================================
  Files         155      155           
  Lines        6494     6494           
=======================================
  Hits         5968     5968           
  Misses        526      526           
Flag Coverage Δ
unittests 91.90% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

pierrelalanne commented 10 months ago

Not sure to understand the context here @GG-HH ? Was this a failing test before the release process ? Should we open a separate issue to find a more long term solution for the isolated_fs to have the expected behavior?

GG-HH commented 10 months ago

Indeed, my description wasn't very explicit (and so is my proposed fix). If you try to run this test alone while having a HMSL token in your cache, the test will fail because it doesn't use an isolated filesystem and the first assert will fail. I did not take time to investigate why the FS isn't isolated.

agateau-gg commented 10 months ago

I think it would be interesting to understand the root cause of the problem here. Where is this cache stored?

agateau-gg commented 10 months ago

Ah, I just had a look at the code and I think I know what's happening. It's because the path to the cache file is initialized globally as a Path. I had a similar issue with other parts of the code. A simple fix is to turn TOKEN_PATH = ... into a get_token_path() function.

GG-HH commented 9 months ago

Thank you @agateau-gg , it indeed fixes the issue. I updated my MR.

I also fix a small typo in a docstring.