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

Convert baseline file to match local filesystem path #573

Closed

Conversation

eddiez9
Copy link
Contributor

@eddiez9 eddiez9 commented Jun 26, 2022

Problem
Currently, detect-secrets doesn't support multi OS environments very well.

If a Windows developer generates a baseline file, Linux developers will always be alerted on new secrets and vice versa.

This is because baseline files have OS specific paths

Windows:
"filename": "src\blah\appsettings
Linux:
"filename": "src/blah/appsettings

When performing new_secrets = secrets - args.baseline you will always get new secrets because the filenames dont match.

Solution
The best solution would probably be to generate baseline files/read files in a consistent manner to always use Unix paths with something like os.path.normpath Doing this however will mean people will need to update their baseline files.

The solution implemented in the PR instead will using os.sep to determine if we are on UNIX or Windows, then replace slashes in filenames for the loaded baseline file (without changing the baseline file). The comparison will then work regardless of if you are on Linux comparing against a windows generated baseline file and vice versa.

@eddiez9
Copy link
Contributor Author

eddiez9 commented Jun 26, 2022

Hey there, requesting any feedback on this PR
@lorenzodb1 @jpdakran

@lorenzodb1
Copy link
Member

Hi @eddiez9, thank you for opening this PR.

Looks like tests are failing. I'd appreciate if you could take a look and get them all passing.

@eddiez9
Copy link
Contributor Author

eddiez9 commented Jun 29, 2022

I've had to change the unit tests as they make assumptions about UNIX pathing wrt. assertions.

To summarize all the changes:
core/potential_secret.py
core/secrets_collection.py

Changed to convert paths to the local file system path seperator for secrets via scans but also via baseline files.

util/path.py
convert to local os path function

tests/audit/audit_test.py
change assertion to use os.path.join instead of hardcoded /

tests/pre_commit_hook_test.py
tests/core/secrets_collection_test.py

change assertions to use os.path.join instead of hardcoded /

tests/core/baselien_test.py
this failing test checks we can read a baseline file and output it exactly the same.
this fails on windows after the path conversion so i've added a check to replace slashes if run on Windows. Another solution for this could potentially have a seperate secrets.baseline.windows file that has windows paths but this complicates pre-commit exclusions

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 this pull request may close these issues.

None yet

2 participants