detect-secrets icon indicating copy to clipboard operation
detect-secrets copied to clipboard

fix not being able to scan from subdirectories

Open Ferada opened this issue 1 year ago • 3 comments

  • Please check if the PR fulfills these requirements
  • [x] Tests for the changes have been added
  • [ ] Docs have been added / updated (which one?)
  • [x] All CI checks are green
  • What kind of change does this PR introduce?

bug fix

  • What is the current behavior?

When running detect-secrets scan from a subdirectory of the git root, no results will be returned (unless --all-files was specified). This doesn't seem to be intentional, as the path handling implies that the current directory may well be in some other place.

Compare detect-secrets scan docs in this repository with (cd docs && detect-secrets scan), the former returns hits for docs/design.md and docs/filters.md, the latter doesn't.

Why is it desirable to have these behave the same? Apart from it being weird if the program silently returns nothing with unexpected arguments, well, for example you might have a monorepo and just want to scan a subdirectory of it. When detect-secrets is being called from another tool, you might not necessarily be aware where the git root is and would have to first navigate there, then pass the path to the subdirectory explicitly, instead of just running detect-secrets scan from where you currently are.

  • What is the new behavior (if this is a feature change)?

Not a new feature, you'll just be able to run detect-secrets scan from a subdirectory and things will still work.

  • Does this PR introduce a breaking change?

Unless someone accidentally relied on no results being returned (I'd bet that'd always be a mistake), this is not a breaking change and might surface some secrets which were previously ignored if the program was called like described above.

Ferada avatar Dec 22 '23 06:12 Ferada

@lorenzodb1 hi, since I've got you here, any thoughts on the problem statement and / or my attempt at fixing it? I've only ran the test cases on Linux, if the failed run there on Mac OS is real, I'll see if I can find a machine to replicate it.

Ferada avatar Jan 04 '24 10:01 Ferada

@Ferada so are you saying the test passes on Linux but fails on MacOS?

lorenzodb1 avatar Jan 23 '24 18:01 lorenzodb1

@Ferada so are you saying the test passes on Linux but fails on MacOS?

@lorenzodb1 I was saying that the GitHub actions output here has two failed runs and several cancelled ones and since I can't restart them I couldn't tell if they were real. However I found a machine to test it on and added one more change to fix up a path problem that, yes, only happens on Mac. I get zero test failures now.

Ferada avatar Jan 24 '24 07:01 Ferada